Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't skip monkeypatches if ActiveSupport present #248

Merged
merged 1 commit into from
Oct 17, 2017

Conversation

dleavitt
Copy link

TLDR: removes two superfluous-seeming checks in core_ext that cause breakage if ActiveSupport is in the require path but not fully loaded.

sidekiq-unique-jobs needs a small number of methods from ActiveSupport. It vendors these methods in case ActiveSupport isn't present.

Currently those methods are only added if ActiveSupport is nowhere Ruby's require path. This breaks in the following cases:

  • ActiveSupport is being used the application but only a subset of methods are included.
  • ActiveSupport is present in the load path but is not being used as part of the application.

This removes the check on whether ActiveSupport is loadable. The solves the above two cases , while still not avoiding re-adding methods in the case that ActiveSupport has already added them.

sidekiq-unique-jobs needs a small number of methods from active support. It contains some vendored methods in case ActiveSupport isn't present.

Currently those methods are only added if ActiveSupport is _nowhere Ruby's the load path_. This breaks in the following cases:
- ActiveSupport is being used the application but only a subset of methods are included.
- ActiveSupport is present in the load path but is not being used as part of the application.

This removes the check on whether ActiveSupport is loadable. This should solve all cases.
@mhenrixon mhenrixon merged commit 8929e8f into mhenrixon:master Oct 17, 2017
@mhenrixon
Copy link
Owner

Thank you for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants