Replace Hash#merge with Utils#deep_merge for connection options#1343
Conversation
iMacTia
left a comment
iMacTia
left a comment
There was a problem hiding this comment.
Thank you @xkwd, this change is very much welcome!
The biggest issue I had with this solution, as I mentioned in my comment on the issue, was that this is effectively backwards-incompatible and might cause surprise for users.
Good news though is that we're in alpha testing for Faraday 2.0 🎉! So I'm happy to merge this is in and simply drop a comment on the UPGRADING.md guide 🙌
The only downside is that if you need this in one of your projects, you'll need to upgrade to Faraday 2.0
| context 'when url is a Hash' do | ||
| let(:conn) { Faraday.new(url: 'http://example.co', headers: { 'CustomHeader' => 'CustomValue' }) } | ||
|
|
||
| before { Faraday.default_connection_options = { headers: { user_agent: 'My Agent 1.2' } } } |
There was a problem hiding this comment.
I just realised there's no rollback of the original value after these test have run 😄.
I'll be fixing this on the main branch, so no worries at all, but please keep this in mind next time you write tests for a global configuration 👍
2 participants
Description
This is an attempt to close the #1248 issue by following the proposed solution
#1in this comment.Prior to this change:
After this change:
The only issue with the proposed solution was that the
deep_mergefromFaraday::Utilsdidn't work out of the box for anOptions StructlikeRequestOptionsandSSLOptionsfromFaraday::ConnectionOptions.