Sidekiq 5 compatibility
jcavalieri opened this issue · 8 comments
Hi @krisleech , thanks for doing a release to rubygems with version 1.1.
I was wondering if you might be up for modifying this line:
https://github.com/krisleech/wisper-sidekiq/blob/master/wisper-sidekiq.gemspec#L22
To this:
spec.add_dependency 'sidekiq', '>=4.0'
To allow for Sidekiq 5.
-John
Yeah, my bad. I'll push a new release. I'm not sure if it works with Sidekiq 4.x, so I think we should go with 'sidekiq', '~> 5.0', what do you think?
Hi
I think I caused some troubles with my PR for sidekiq 5.x compatibility 🙊
.set API was actually introduced in 4.1 - references and I forgot to check that before I suggested the solution, my bad!
But given the fact that 4.1 was released on January 28, 2016 plus very easy upgrade process from 3.x I would just drop support for everything lower than that - IMO it's totally not worth the effort - wdyt ❓
@emq thanks for the input. These things happen 😄 We should really test against each version of Sidekiq to be fair.
So would I be right in thinking that master branch should work with >= 4.1?
ok, I'll make the change on the pr for 4.1
@krisleech yeah, >= 4.1 should do the job just fine, I think it's very doubtful that api will change any time soon as it was added for activejob's compatibility and make it's way to 5.x, so probably we can safely rely on it
If you want to support lower versions of sidekiq if will probably have to do some nasty if-ology 😕 but again - personally I don't see much value in it, unless you use older versions of sidekiq anywhere 🤷♂️
new PR here
#23
@emq Thanks. Anyone who is using older Sidekiq can use 0.0.1 anyway, which works for all versions up to the point #delay was removed by default.
PR merged and 1.2.0 released 👍