Make deprecation warning actionable

wspurgin / rspec-sidekiq

RSpec for Sidekiq

https://github.com/wspurgin/rspec-sidekiq

Other

655 stars 132 forks source link

Make deprecation warning actionable #126

Closed trev closed 7 years ago

trev commented 7 years ago

As it currently stands, this deprecation warning is un-actionable. The warning suggests a way to "resolve" the deprecation but that's not exactly right.

trev commented 7 years ago

@wpolicarpo @packrat386 @philostler Sorry for the ping but the PRs are looking stale. Also #110 could be merged as part of this. Happy to help maintain if you guys are too busy.

coveralls commented 7 years ago

Coverage decreased (-5.8%) to 91.389% when pulling e9b119030ea7a8b9bd3316399626c99ea1ffde9e on trev:actionable-deprecation into 4f6da310f2311aa2d5bbea983bd17776d041c61a on philostler:develop.

coveralls commented 7 years ago

Coverage decreased (-5.8%) to 91.389% when pulling e9b119030ea7a8b9bd3316399626c99ea1ffde9e on trev:actionable-deprecation into 4f6da310f2311aa2d5bbea983bd17776d041c61a on philostler:develop.

coveralls commented 7 years ago

Coverage decreased (-5.8%) to 91.389% when pulling 4e8c48d6ec9cbaccbe20341fb8606e828db800f0 on trev:actionable-deprecation into 4f6da310f2311aa2d5bbea983bd17776d041c61a on philostler:develop.

coveralls commented 7 years ago

Coverage decreased (-5.8%) to 91.389% when pulling 4e8c48d6ec9cbaccbe20341fb8606e828db800f0 on trev:actionable-deprecation into 4f6da310f2311aa2d5bbea983bd17776d041c61a on philostler:develop.

coveralls commented 7 years ago

Coverage decreased (-5.8%) to 91.389% when pulling b8995b05e9ad9fa02794ca479ba0e2c7277da99d on trev:actionable-deprecation into 4f6da310f2311aa2d5bbea983bd17776d041c61a on philostler:develop.

coveralls commented 7 years ago

Coverage increased (+0.3%) to 97.5% when pulling 7433d0ffd96a3aaa9e6def8694ee18cad0811c47 on trev:actionable-deprecation into 4f6da310f2311aa2d5bbea983bd17776d041c61a on philostler:develop.

trev commented 7 years ago

The tests are failing on Ruby < 2.2 because Sidekiq 5+ requires Ruby >= 2.2.2.

What I suggest we do is:

Adjust this PR so that it's only using Sidekiq < 5.0.0 and only has the deprecation change and patch version bump: https://github.com/philostler/rspec-sidekiq/pull/126/commits/e9b119030ea7a8b9bd3316399626c99ea1ffde9e & https://github.com/philostler/rspec-sidekiq/pull/126/commits/b8995b05e9ad9fa02794ca479ba0e2c7277da99d
Create a new PR that removes the deprecation warning and makes have_enqueued_sidekiq_job the default, update gem dependencies to use latest Sidekiq, remove support for Ruby < 2.2.2 and finally bump version to 4.0.0

If that sounds good, let me know and I'll affect the changes.

trev commented 7 years ago

@wpolicarpo @packrat386 @philostler Sorry to ping again. A 👍 or 👎 would be great if my previous message sound reasonable.

tisba commented 7 years ago

This looks good, thanks @trev!

Anything we can do/help with to get this shipped?

trev commented 7 years ago

Hey @tisba, I'm happy to affect the changes as it's a quick easy job but I can't get any of the maintainers to get back to me. I've emailed them as well. @wpolicarpo @packrat386 @philostler -- ping again :)

packrat386 commented 7 years ago

On that note, do you have any objection to me just merging #128, it seems like those 2 PRs do the same thing.

trev commented 7 years ago

@packrat386 I took the best of both worlds as his PR was missing stuff:

This PR can be closed.

trev commented 7 years ago

Closing in favour of #130 #131 & #132 which once merged together will result in passing tests and resolve multiple issues.