search

amitree / delayed_job_recurring

Extends delayed_job to support recurring jobs
MIT License
84 stars 33 forks source link

delayed_job 3.x #3

Closed iamvery closed 9 years ago

iamvery commented 9 years ago

I recently ran into some parallel computing issues with the clockwork gem. After some research we decided to use your gem to solve our problems by leaning on delayed jobs (which we already have in the app).

I came upon a few things that I thought would be worth adjusting in the gem. I hope you don't mind! Of course, I'm happy to hear your feedback and make any changes necessary to meet your liking :+1:

This change may be a little more controversial. As I mentioned in #2, our app is getting a little old. Unfortunately we're locked at Rails 3 for the time being. That makes our dependency tree incompatible with your library. Luckily it wasn't too difficult to make things happy! Much credit to delayed_job for not breaking too much compatibility between versions 3 and 4.

The change to the spec_helper is a little weird. To be honest, I'm not entirely sure why this line isn't needed (and I'm 100% for being explicit about things). Unfortunately Rails 3 doesn't have the abstraction, so in order to get the tests running in Rails 3, the line had to be removed. I did verify that tests still pass with Rails 4. Maybe you should consider adding Travis CI to your project?

What do you think? :blush:

iamvery commented 9 years ago

@afn I updated the PR to remove the suite-breaking commit :+1: