Skip to content

Conversation

@arnoldasgudas
Copy link

It would be really great to have interface for RecurringJobManager so that it would be able to mock it in unit tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other overloads are just a syntax sugar for this method, and shouldn't be included into the interface. Plus, timeZone and queue parameters were added since the initial release of this class. Therefore I'm expecting that there will be some other parameters as well.

Since we can't modify interface methods without introducing breaking changes, I think there should be the RecurringJobOptions class that will contain TimeZone and Queue properties (and other after some time), and the final signature that should go to the interface will be like this:

void AddOrUpdate(string recurringJobId, Job job, string cronExpression, RecurringJobOptions options);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment. I've added RecurringJobOptions class and updated IRecurringJobManager interface.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method shouldn't be placed to an interface. This is an overload that may be implemented as an extension method that calls the method above with the default options. This will result in a much simpler interface.

@odinserj odinserj added this to the 1.6.0 milestone Mar 23, 2016
odinserj added a commit that referenced this pull request Apr 5, 2016
* A couple of methods are extensions now to be able to use them from
`IRecurringJobManager`.
* Add null checks to `RecurringJobOptions` class
* Add some unit tests
@odinserj
Copy link
Member

odinserj commented Apr 5, 2016

I've merged this PR into the dev branch, please see commits 141cbb3 and 9115252. Also I've made the following changes. Thank you for the PR!

  • Remove overloads of the AddOrUpdate method.
  • A couple of methods are extensions now to be able to use them from
    IRecurringJobManager.
  • Add null checks to RecurringJobOptions class
  • Add some unit tests

@odinserj odinserj closed this Apr 5, 2016
@odinserj odinserj mentioned this pull request Jul 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants