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

Add support for Ruby 3.2.0 in Faraday v1.x #1483

Merged
merged 3 commits into from Jan 18, 2023

Conversation

timrogers
Copy link

@timrogers timrogers commented Jan 18, 2023

When adding a middleware that receives keyword arguments in the constructor, the call from DependencyLoader#new fails because the method is not defined using ruby2_keywords.

This adds the required ruby2_keywords declaration to DependencyLoader#new, fixing the tests and getting Faraday v1.x working with Ruby 3.2.0.

Fixes #1479.

When adding a middleware that receives keyword arguments in the
constructor, the call from `DependencyLoader#new` fails because
the method is not defined using `ruby2_keywords`.

This adds the required `ruby2_keywords` declaration to
`DependencyLoader#new`, fixing the tests and getting Faraday v1.x
working with Ruby 3.2.0.

Fixes lostisland#1479.
@timrogers timrogers marked this pull request as ready for review January 18, 2023 09:50
@timrogers
Copy link
Author

There are some failing tests here - which at least according to my machine - seem to exist before my changes. Any ideas? 🙏

@iMacTia
Copy link
Member

iMacTia commented Jan 18, 2023

Apologies about that @timrogers, let me take a look

@timrogers
Copy link
Author

Apologies about that @timrogers, let me take a look

Thank you ❤️ Apologies if my comment came across as a bit demanding - on re-reading it, it wasn't as kind as it could have been. I know that maintaining an open-source project is a thankless task!

@iMacTia
Copy link
Member

iMacTia commented Jan 18, 2023

Not at all! I'm just surprised as to why this would start failing all of a sudden 🤔
I'm testing it locally simply switching ruby with RVM and it works fine, which makes me think this might be due to some dependency breaking on a recent update

@timrogers
Copy link
Author

Not at all! I'm just surprised as to why this would start failing all of a sudden 🤔 I'm testing it locally simply switching ruby with RVM and it works fine, which makes me think this might be due to some dependency breaking on a recent update

Yeah. It definitely seems that it was green when the Actions CI ran at the time when it was merged. I too suspect a dependency update which is allowed by the gemspec/Gemfile.

@iMacTia
Copy link
Member

iMacTia commented Jan 18, 2023

OK I managed to pinpoint it to rack 3.0, trying to understand what changed and if/what we should fix.

@timrogers
Copy link
Author

@iMacTia Thanks! This is looking good now.

@timrogers
Copy link
Author

@iMacTia Thanks for the quick review and merge. Looking forward to this getting shipped as the issue was noticed in my project, Restforce.

@iMacTia
Copy link
Member

iMacTia commented Jan 18, 2023

Thank you @timrogers for fixing it 🙏 !
This is now released as v1.10.3

@olleolleolle olleolleolle linked an issue Jan 18, 2023 that may be closed by this pull request
bolshakov pushed a commit to toptal/faraday that referenced this pull request Mar 22, 2023
* Run tests against Ruby 3.2

* Declare `DependencyLoader#new` with `ruby2_keywords` to fix Ruby 3.2.0

When adding a middleware that receives keyword arguments in the
constructor, the call from `DependencyLoader#new` fails because
the method is not defined using `ruby2_keywords`.

This adds the required `ruby2_keywords` declaration to
`DependencyLoader#new`, fixing the tests and getting Faraday v1.x
working with Ruby 3.2.0.

Fixes lostisland#1479.

Co-authored-by: Matt <iMacTia@users.noreply.github.com>
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.

ArgumentError due to missing ruby2_keywords on Ruby 3.2.0
2 participants