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

Fix 2.7 deprecation warning in middleware/stack #2123

Merged

Conversation

Legogris
Copy link
Contributor

@grape-bot
Copy link

grape-bot commented Oct 16, 2020

1 Warning
⚠️ There’re library changes, but not tests. That’s OK as long as you’re refactoring existing code.

Generated by 🚫 danger

@dblock dblock merged commit 552e01c into ruby-grape:master Oct 16, 2020
@dblock
Copy link
Member

dblock commented Oct 16, 2020

Thanks, I think this was the last one.

@Legogris
Copy link
Contributor Author

Thanks, I think this was the last one.

Thanks for merging! Almost ;)
#2121

@Legogris Legogris deleted the fix-2.7-deprecation-middleware-stack branch October 17, 2020 08:13
@Legogris Legogris mentioned this pull request Oct 19, 2020
@eregon
Copy link
Contributor

eregon commented Nov 16, 2020

This is unfortunately not correct, *args, **opts-delegation is not correct in some cases on 2.7.
The only correct way to do delegation on Ruby 2.7 is using ruby2_keywords + *args-delegation.
This is also what the Rails ActionDispatch::MiddlewareStack does (https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/stack.rb, which is very similar).

See https://eregon.me/blog/2019/11/10/the-delegation-challenge-of-ruby27.html for why it's broken on 2.7,
and as you mention above https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/

A compatible delegation that works on Ruby 2.6, 2.7 and Ruby 3
In short: use Module#ruby2_keywords again.

FWIW, I'm coming from oracle/truffleruby#2159

@dblock
Copy link
Member

dblock commented Nov 16, 2020

Interesting. Can we get rid of all of this complexity?

@eregon
Copy link
Contributor

eregon commented Nov 17, 2020

Yes, if you use ruby2_keywords you only need a few ruby2_keywords calls on methods accepting *args.

@dblock
Copy link
Member

dblock commented Nov 17, 2020

@eregon care to PR a fix?

@eregon
Copy link
Contributor

eregon commented Nov 19, 2020

#2132

@Legogris
Copy link
Contributor Author

@eregon Derp, great that you caught this!

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.

4 participants