-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Require main active_support lib before any of its extension definitions #2206
Conversation
This comment has been minimized.
This comment has been minimized.
thanks, it certainly makes sense
if I understood correctly it was recommended with active_support = 5.0 also or even earlier. could you please update the changelog? |
01bc889
to
f43a8f0
Compare
Done @dm1try |
It seems to be the recommended way with active_support. See https://guides.rubyonrails.org/active_support_core_extensions.html#cherry-picking-a-definition Fix ruby-grape#2205. Signed-off-by: Baptiste Courtois <b.courtois@criteo.com>
f43a8f0
to
f98ea2c
Compare
@dblock @dm1try I think a new release is required for this one. this could break (at least on the-benchmarker/web-frameworks#4896) |
@waghanza makes sense, I'll do it on this weekend if nobody else picks it up first |
I had started on last weekend but then realized that "ruby-head" test targets are red and it's not clear how important it is. So the release was postponed for a while. |
released @dblock could you please send a message to the google group, it looks like I've lost the access to my account. |
Looks like you got it back or sent it from another one? I hit approve on the release email. Thanks for making a release! |
yeah, I've restored some account) |
It seems to be the recommended way with active_support >= 7.0.
See https://guides.rubyonrails.org/active_support_core_extensions.html#cherry-picking-a-definition
Fix #2205.