search

lostisland / faraday

Simple, but flexible HTTP client library, with support for multiple backends.
https://lostisland.github.io/faraday
MIT License
5.71k stars 972 forks source link

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

Closed timrogers closed 1 year ago

timrogers commented 1 year ago

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 https://github.com/lostisland/faraday/issues/1479.

timrogers commented 1 year ago

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

iMacTia commented 1 year ago

Apologies about that @timrogers, let me take a look

timrogers commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

timrogers commented 1 year ago

@iMacTia Thanks! This is looking good now.

timrogers commented 1 year ago

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

iMacTia commented 1 year ago

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