-
Notifications
You must be signed in to change notification settings - Fork 150
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
Make socksify and rack gem dependencies optional #225
Make socksify and rack gem dependencies optional #225
Conversation
9e25a26
to
650b037
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
83f391e
to
2ed09a9
Compare
I am frantically pushing APPROVE AND RUN |
8f41b40
to
e9057fd
Compare
spec/httpi/httpi_spec.rb
Outdated
@@ -294,7 +292,7 @@ | |||
end | |||
|
|||
HTTPI::Adapter::ADAPTERS.each do |adapter, opts| | |||
unless (adapter == :em_http && RUBY_VERSION =~ /1\.8/) || (adapter == :curb && RUBY_PLATFORM =~ /java/) | |||
unless (adapter == :em_http || adapter == :curb) && RUBY_PLATFORM =~ /java/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned, a platform-to-adapters-supported-on-that-platform could be interesting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this was a quick hack to get it passing since it's already 1AM lol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All in due time, that's a note for future PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
So, I think this is at the place where we need to look at things we can extract from it, and offer to the default branch. A way forward with this big a change is a new major version, which allows to shed old version support etc. |
In addition to the earlier things fixed, the CI issue is also fixed by this. |
a797a27
to
e777d2f
Compare
Thank you for your work on this. |
This has now been released as 3.0.0. https://github.com/savonrb/httpi/releases/tag/v3.0.0 |
Thank you so much. |
Fixes #224. Fixes #205.