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

Move out Net::HTTP adapter #1222

Merged
merged 3 commits into from
Dec 29, 2020
Merged

Move out Net::HTTP adapter #1222

merged 3 commits into from
Dec 29, 2020

Conversation

iMacTia
Copy link
Member

@iMacTia iMacTia commented Dec 23, 2020

Description

Removes Net::HTTP adapter from Faraday codebase and adds it as a dependency instead.
Replaces #1195

Additional Notes

This change should be completely backwards-compatible.
The dependency should be removed in Faraday v2.0

JanDintel and others added 2 commits October 29, 2020 20:28
The Net::HTTP adapter is extracted into its own gem. The idea behind this change is to create an ecosystem of different packages that you can use with Faraday, so that Faraday doesn’t have to ship with the packages itself.

For now all the Net::HTTP tests are still in place to verify that everything is working correctly.
@iMacTia
Copy link
Member Author

iMacTia commented Dec 23, 2020

Just waiting for lostisland/faraday-net_http#2 to be merged to sync the 2 repos up again, then bump the faraday-net_http version in the gemspec and merge this in 🎉 !

faraday.gemspec Outdated
@@ -15,6 +15,7 @@ Gem::Specification.new do |spec|

spec.required_ruby_version = '>= 2.4'

spec.add_dependency 'faraday-net_http', '~> 0.0.6'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, perhaps we should start it at 1.0?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like, the code is what we use in this repo.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I have already bumped faraday-net_http version to 1.0, BUT I realised @JanDintel is the only owner of the gem in Rubygems at the moment! So I can't publish it right now.
@JanDintel could you please add me and @olleolleolle as owners as well so we can publish new versions?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iMacTia and @olleolleolle I've added you as owners to the gem! You should have full permission to publish a new version.

@iMacTia I noticed that you still need to set-up MFA on Rubygems though :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JanDintel thank you! Will release it now then, thanks a lot for all the work 🎉

@iMacTia I noticed that you still need to set-up MFA on Rubygems though :)

Yes, that's a looooooong story: rubygems/rubygems.org#2500

Copy link
Member

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love that the hackathon at Rossconf had this cool outcome!

@iMacTia iMacTia merged commit e41668e into master Dec 29, 2020
@iMacTia
Copy link
Member Author

iMacTia commented Dec 29, 2020

It's done! Massive thanks to @JanDintel for doing the bulk of the work 🎉

@olleolleolle olleolleolle deleted the move-out-net-http branch December 29, 2020 11:30
@rmm5t
Copy link

rmm5t commented Jan 11, 2021

This causes a circular dependency issue.

faraday requires faraday/net_http. faraday/net_http requires faraday. wash. rinse. repeat.

/path/vendor/bundle/ruby/2.5.0/gems/faraday-net_http-1.0.0/lib/faraday/net_http.rb:3: warning: loading in progress, circular require considered harmful - /Users/mcgear/work/account-service/vendor/bundle/ruby/2.5.0/gems/faraday-1.3.0/lib/faraday.rb
                from /path/vendor/bundle/ruby/2.5.0/gems/faraday-1.3.0/lib/faraday.rb:30:in  `<top (required)>'
                from /path/vendor/bundle/ruby/2.5.0/gems/faraday-1.3.0/lib/faraday.rb:30:in  `require'
                from /path/vendor/bundle/ruby/2.5.0/gems/faraday-net_http-1.0.0/lib/faraday/net_http.rb:3:in  `<top (required)>'

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