Allow http 5-x

[mgrunberg]

http 5.0.0 has released a few days ago.

The PR adds support for 5.x.

http drop Ruby 2.3 and 2.4 support (httprb/http#571 and this commit). That's why I'm using 4.x on CI when ruby is lower than 2.5.0.

[jrochkind]

Does http 5.x not work with ruby 2.4 and 2.3?

Ah, i see the change log for http says it's dropped ruby 2.3.0 support with http 5.0. Is there also an issue with ruby 2.4, @mgrunberg ?

[jrochkind]

This doesn't actually work to do what you want, I'm afraid.

Logic in a gemspec is executed at build time, so which of these gets included will depend on what version of ruby the releaser was using.

There isn't any way to do conditional logic in a gemspec like this that applies at runtime/execution time different for different apps.

However... I'm confused that this is only a development dependency not an actual runtime dependency... maybe this is ok? Is this really only about CI? But it's very confusing/misleading, and will result in a gem released to rubygems whose development_dependency for http depends on what version of ruby the releaser was running...

I am not sure what to do, but I think ideally there is a different solution than this. Or at least some very explicit comments. Because this sort of thing is an error people make in gemspecs a lot, and 99% of the time it is an error, and causes big problems.

[mgrunberg]

This doesn't actually work to do what you want, I'm afraid.

Logic in a gemspec is executed at build time, so which of these gets included will depend on what version of ruby the releaser was using.

There isn't any way to do conditional logic in a gemspec like this that applies at runtime/execution time different for different apps.

However... I'm confused that this is only a development dependency not an actual runtime dependency... maybe this is ok? Is this really only about CI? But it's very confusing/misleading, and will result in a gem released to rubygems whose development_dependency for http depends on what version of ruby the releaser was running...

I am not sure what to do, but I think ideally there is a different solution than this. Or at least some very explicit comments. Because this sort of thing is an error people make in gemspecs a lot, and 99% of the time it is an error, and causes big problems.

I will try to do my best to explain what lead me to this solution.

down has several backends (readme). You don't always need http gem to use down. If you use the default backend, there is no point in adding http to your system. This is by, as I understand, it's not a gemspec dependency.

It's needed as a development_dependency to test Down::Http backend. But also because some specs use it to build the expected result (example) and because test_helper uses it to wait for a service.

Since http drop ruby 2.3 and 2.4 support down could do the same. That's fair. But users running old rubies (2.3 and 2.4 have reached EOL) can still use Down::Http if they use 4.x version. Drop old rubies support is not a requirement to use http 5.0, so I took the conservative alternative and let specs run with 4.x on old rubies (asserting that this works).

Why the condition logic is in the gemspec and not in another place (Gemfile for example)? Because there are already some development_dependencies handled that way.

[mgrunberg]

Does http 5.x not work with ruby 2.4 and 2.3?

Ah, i see the change log for http says it's dropped ruby 2.3.0 support with http 5.0. Is there also an issue with ruby 2.4, @mgrunberg ?

They also drop support for ruby 2.4. It's not on the changelog but this is the commit where they bump 2.4 to 2.5.
5.0 gemspec ruby_version is >= 2.5.

[jrochkind]

OK thanks, I'm going to file a PR to their CHANGELOG adding that then.... seems important to be in changelog.

Not sure the right way to handle this over here, as per my comment I'm concerned about the way you are doing it, but maybe I'm wrong? Curious for maintainer opinion.

[jrochkind]

httprb/http#666

[mgrunberg]

OK thanks, I'm going to file a PR to their CHANGELOG adding that then.... seems important to be in changelog.

Thanks for that

Not sure the right way to handle this over here, as per my comment I'm concerned about the way you are doing it, but maybe I'm wrong? Curious for maintainer opinion.

I'm answering your other comment but I need some time to order my ideas and do a proper explanation 😄

[janko]

Looks good, thanks for the patch!