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

Drop Ruby 2.4/2.5/2.6 support #149

Merged
merged 1 commit into from
Jul 6, 2023
Merged

Conversation

bastelfreak
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (0921dff) 94.93% compared to head (810635c) 94.93%.

❗ Current head 810635c differs from pull request most recent head 0857fbf. Consider uploading reports for the commit 0857fbf to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #149   +/-   ##
=======================================
  Coverage   94.93%   94.93%           
=======================================
  Files           2        2           
  Lines         158      158           
=======================================
  Hits          150      150           
  Misses          8        8           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@alexjfisher
Copy link
Member

Echoing voxpupuli/puppet-blacksmith#109 (comment) I'd only drop 2.4 for now as I don't think you're helping anyone by being overly aggressive without tangible benefits.

Puppet 6 (with Ruby 2.5) has only just gone EOL. IMO it's not helping people upgrading from 6 to 7 if we drop ruby 2.5 support in all the core testing gems.

@bastelfreak
Copy link
Member Author

It might not help them, but it doesn't hurt them. The gem works fine with Ruby 2.5/Puppet 6 and with Ruby 2.7/Puppet 7. I think it even works already with Puppet 8. So why should we keep support for a ruby version that left normal maintenance mode at 2020-04-01 and does not even get security updates since 2021-04-05?

And as mentioned in the blacksmith PR, we need to bump the minimal Ruby version, at least for acceptance tests, to 2.7 anyways with the next major beaker release.

@alexjfisher
Copy link
Member

Will this limit its inclusion with the PDK?

@bastelfreak
Copy link
Member Author

I don't think so. Why should it limit this? pdk-tempaltes just needs to pin to the current major version of rspec-puppet-facts. Which it already does: https://github.com/puppetlabs/pdk-templates/blob/2.7.3/config_defaults.yml#L587-L588

@alexjfisher
Copy link
Member

But it will then miss out on the legacy facts stuff we've started working on, and I think that would be a shame and hurt users.

@bastelfreak
Copy link
Member Author

We don't need to merge this now, we can first merge #143 and do a final minor release.

@alexjfisher
Copy link
Member

Until supporting ruby 2.5 is actually holding us back (limiting what version of rubocop we can use, or some new ruby feature we would like), I'm strongly in favour of not advancing past the versions of ruby that ship with the PDK.

@ekohl
Copy link
Member

ekohl commented Mar 17, 2023

I agree with @alexjfisher that - besides beaker - dropping Ruby 2.5 is a tad too aggressive for now.

@alexjfisher
Copy link
Member

Also FWIW puppetserver 7's jruby is only ruby 2.6 compatible.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

@alexjfisher I'm not sure that compatibility limitation really relevant here, unless you intend to really run the test suite with Ruby 2.6.

In #150 we can see that Ruby 2.5 is holding us back so I'm ok with raising it to 2.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants