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

Update information on supported Ruby versions #951

Merged
merged 5 commits into from
Mar 16, 2022
Merged

Conversation

chrisseaton
Copy link
Member

Let's commit to 2.4.10 as a minimum.

That lets us run on:

  • macOS system Ruby
  • Previous LTS Ubuntu
  • Debian old stable

That's what we're actually testing in practice, so what we can genuinely claim. It's two major versions of unmaintained.

I'll say we support latest JRuby and TruffleRuby, which seems to be the case in practice based on what's being test. We should maybe add more JRuby versions to CI and keep supporting a few old versions. @headius for any opinion.

I'll leave the Rubinius code in for now, because I don't want to rip out quite a lot of code in this release, but I've warned off that it will likely go in the future.

@jeremyevans

@chrisseaton chrisseaton requested a review from eregon March 15, 2022 14:24
@jeremyevans
Copy link

In general, the required_ruby_version should be set to the minimum Ruby version that will run the gem, not the minimum version you support. Otherwise you are just hurting users of older Ruby versions. If the library runs with Ruby 1.9.3, even if you don't support it, I would just change the documentation and leave required_ruby_version alone. Another way to handle this is to start using features added in Ruby 2.4, such as Regexp#match?.

Assuming the gem actually does require Ruby 2.4, this looks mostly good to me. Ruby 2.4 minimum version is what we are targeting in Rack 3 (for Regexp#match? support). However, unless there are specific reasons to require 2.4.10+, I would change it to just >= 2.4.

@chrisseaton
Copy link
Member Author

In general, the required_ruby_version should be set to the minimum Ruby version that will run the gem, not the minimum version you support.

Aren't these the same thing? If we aren't running a Ruby version in CI then I don't know if the gem will run on it or not.

@jeremyevans
Copy link

In general, the required_ruby_version should be set to the minimum Ruby version that will run the gem, not the minimum version you support.

Aren't these the same thing? If we aren't running a Ruby version in CI then I don't know if the gem will run on it or not.

If you don't know, be optimistic, not pessimistic, and leave the required version at 1.9.3. If you get a report that it doesn't work on an unsupported version, then bump required_ruby_version at that point. required_ruby_version should indicate you are 100% sure the gem doesn't work in earlier versions, because of the Ruby features it uses.

If you want, I can test CI for you on earlier Ruby versions (GitHub CI supports back to 1.9.3) to tell you when it actually fails.

@jeremyevans
Copy link

Looks like it is safe to set required_ruby_version to 2.2. Personally, I would keep CI with 2.2, and if you only want to support 2.4+, start using Ruby 2.4 features, see CI fail with them, then drop the failing versions from CI and bump required_ruby_version.

@eregon
Copy link
Collaborator

eregon commented Mar 15, 2022

If the library runs with Ruby 1.9.3, even if you don't support it, I would just change the documentation and leave required_ruby_version alone.

I agree with most of what you said except this part, I think it's important and valuable to set required_ruby_version.
If the next release install installs on some old Ruby version but does not work on that Ruby, it's better to have the required_ruby_version so then an older concurrent-ruby version is installed instead, and that version should actually work with that old Ruby (whether using bundle/gem install and no gem version requirement).

IMHO 2.4 is reasonable.
2.2 might work currently but it seems likely it might need extra efforts for it at some point.
Because 2.2 is quite difficult to install or troubleshoot locally so I'd drop it now (e.g., it needs libssl 1.0 which is EOL).
For the same reason I'd drop 2.4 and require 2.5.
(https://github.com/ruby/setup-ruby#supported-versions)

I think a key method we need for concurrent-ruby is Etc.nprocessors, that's available since 2.2, and would simplify https://github.com/ruby-concurrency/concurrent-ruby/blob/master/lib/concurrent-ruby/concurrent/utility/processor_counter.rb a lot.

@jeremyevans
Copy link

If the library runs with Ruby 1.9.3, even if you don't support it, I would just change the documentation and leave required_ruby_version alone.

I agree with most of what you said except this part, I think it's important and valuable to set required_ruby_version. If the next release install installs on some old Ruby version but does not work on that Ruby, it's better to have the required_ruby_version so then an older concurrent-ruby version is installed instead, and that version should actually work with that old Ruby (whether using bundle/gem install and no gem version requirement).

The simplest fix here is to CI test the older version even if not officially supported. As soon as CI fails for the older version, drop it from CI and bump required_ruby_version.

I'm not advocating supporting older rubies indefinitely. I'm advocating for setting required_ruby_version correctly, as in, required_ruby_version states which Ruby version is actually required to run the library. It's perfectly fine to me to drop support for older Ruby versions and bump required_ruby_version as soon as you are using the features in the newer Ruby version.

Don't let older Ruby versions hold you back, feel free to use features from newer Ruby versions. However, if your gem will run on older Ruby versions, don't artificially restrict users of those Ruby versions from using the library. required_ruby_version should not be a barrier, it should be a guardrail, only preventing users from wasting their time when you know the library will not work.

IMHO 2.4 is reasonable. 2.2 might work currently but it seems likely it might need extra efforts for it at some point. Because 2.2 is quite difficult to install or troubleshoot locally so I'd drop it now (e.g., it needs libssl 1.0 which is EOL). For the same reason I'd drop 2.4 and require 2.5. (https://github.com/ruby/setup-ruby#supported-versions)

2.2 works just fine in CI. If you commit something that works on 2.4/2.5 and breaks on 2.2 in CI, bump required_ruby_version and drop that Ruby version from CI.

I think a key method we need for concurrent-ruby is Etc.nprocessors, that's available since 2.2, and would simplify https://github.com/ruby-concurrency/concurrent-ruby/blob/master/lib/concurrent-ruby/concurrent/utility/processor_counter.rb a lot.

CI currently fails for Ruby <2.2, so it looks like 2.2 is already required, at least to run the tests.

@chrisseaton
Copy link
Member Author

I think I will leave 2.2 and above running in CI, and mark the gem as supporting 2.2 and above, as that is the factual reality.

Can re-address in a major release.

@eregon
Copy link
Collaborator

eregon commented Mar 16, 2022

I think I will leave 2.2 and above running in CI, and mark the gem as supporting 2.2 and above, as that is the factual reality.

Agreed.

Can re-address in a major release.

IMHO no need for a major release, that's the point: required_ruby_version would automatically prevent installing a newer concurrent-ruby which needs a newer Ruby version on an older Ruby version. Specifically this requirement is an issue as major releases of a gem are rare and lots of work, and completely unrelated to the pace of CRuby releases, so they should IMHO not be tight together.

@chrisseaton chrisseaton merged commit e89530e into master Mar 16, 2022
@chrisseaton chrisseaton deleted the ruby-versions branch March 16, 2022 14:41
@eregon
Copy link
Collaborator

eregon commented Mar 18, 2022

One argument for requiring 2.3 minimum is Thread#{name,name=} support.
But it seems not hard to keep working around the lack of it in 2.2.

@eregon
Copy link
Collaborator

eregon commented Dec 21, 2022

We will require Ruby 2.3 in the next release: #975

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