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

Bump required ruby version to v2.4.0 #536

Merged
merged 1 commit into from Mar 30, 2021

Conversation

cgunther
Copy link
Contributor

a9137ac introduced a use of Hash#transform_values in Listen::Record#dir_entries, however that was only added to Ruby in v2.4.

This can lead to runtime errors when run on an older version of Ruby, such as 2.2.7, which the gemspec previously implied was supported.

It looks like CI only runs on Ruby 2.5+, so an argument could also be made the gemspec should follow suit:

- 2.5
- 2.6
- 2.7

Bumping to v2.4 was the simplest to resolve the immediate problem I was facing running an old project on Ruby v2.3.4 while I work to upgrade it.

@ColinDKelley
Copy link
Collaborator

ColinDKelley commented Mar 30, 2021

@cgunther Thanks for reporting this bug. I fear it might cause problems in other projects if we narrowed the supported Ruby version, though. For example, with Rails itself.

I believe we can get equivalent behavior to

      subtree.transform_values do |values|
        # only get data for file entries
        values.has_key?(:mtime) ? values : {}
      end

with

      subtree.each_with_object({}) do |(key, values), result|
        # only get data for file entries
        result[key] = values.has_key?(:mtime) ? values : {}
      end

Look good?

I'd like to add Ruby 2.2 to the CI matrix as well, if we can.

@ColinDKelley
Copy link
Collaborator

I'm trying this branch to see how hard it is to get 2.2 tests passing: #537

Looks like rubocop 0.91 requires ruby 2.4...

@ColinDKelley
Copy link
Collaborator

ColinDKelley commented Mar 30, 2021

Hmm, from that other PR, I'm convinced that listen is not even close to working with ruby 2.2 or 2.3..

So I agree that 2.4 is right place to draw the line, as you have in this PR. Can you please change this PR to add 2.4 to the ruby: versions in .github/workflows/development.yml?

a9137ac introduced a use of Hash#transform_values in
Listen::Record#dir_entries, however that was only added to Ruby in
[v2.4](https://github.com/ruby/ruby/blob/v2_4_0/NEWS#core-classes-updates-outstanding-ones-only-).

This can lead to runtime errors when run on an older version of Ruby,
such as 2.2.7, which the gemspec previously implied was supported.

Also expand CI to match and test on Ruby v2.4.
@cgunther
Copy link
Contributor Author

Updated!

I'd agree there's little value in supporting Ruby < v2.4. In my little analysis, listen v3.3.2 was the first to use this new Ruby 2.4 feature and that was released in Nov 2020, so if in 4 months no one else complained about an incompatibility with Ruby < 2.4, it may not be much of an issue. From a Ruby perspective, v2.4 reached end-of-life in April 2020, so it's debatable how much effort should go into supporting it, but this PR is at least a step in the right direction.

@ColinDKelley ColinDKelley merged commit 1243d92 into guard:master Mar 30, 2021
@cgunther
Copy link
Contributor Author

Thanks for the quick merge!

@cgunther cgunther deleted the gemspec-ruby-2.4 branch March 30, 2021 20:27
@ColinDKelley
Copy link
Collaborator

Released in listen v3.5.1.

Thanks again for reporting and fixing this!

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.

None yet

2 participants