-
Notifications
You must be signed in to change notification settings - Fork 38
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 puppet_forge, which requires Ruby 2.6+ #94
Conversation
Regarding the test failures - I think I can get the tests passing by pinning |
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.
This is going to be a major version bump. Let's use that to be more aggressive. Ruby 2.6 is already EOL and Puppet doesn't use it either.
librarian-puppet.gemspec
Outdated
@@ -15,7 +15,7 @@ Gem::Specification.new do |s| | |||
automatically pulling in modules from the forge and git repositories with | |||
a single command.' | |||
|
|||
s.required_ruby_version = '>= 2.4.0', '< 4' | |||
s.required_ruby_version = '>= 2.6.0', '< 4' |
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.
If we're dropping old versions, I'd prefer to move it straight to 2.7+
s.required_ruby_version = '>= 2.6.0', '< 4' | |
s.required_ruby_version = '>= 2.7', '< 4' |
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.
I think this means removing Puppet 5 from the test matrix - currently that's excluded from tests on 2.7 and 3.0 and it's EOL anyway so it doesn't feel unreasonable to drop support for that either, given a major version bump.
I'll add the necessary changes.
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.
Puppet 5 used to be shipped with Puppet 2.4 anyway. It happened to run on Ruby 2.6, but that was not what the AIO version was. And as you say, it's EOL. Even Puppet 6 is going EOL soon.
I just realized I already opened #93 a while back. This just takes it a step further in terms of Ruby versions, but I already did a lot of the additional work like README. I'm going to revisit my PR after lunch and see what we should do. |
This is ahead of adding support for `puppet_forge` >= 4.0.0, which in turn adds support for `faraday` 2.x which is its turn requires at least Ruby 2.6.
This updates `puppet_forge`, bringing in an updated `faraday` gem in order to remove some deprecation warnings. Fixes voxpupuli#91
Both Ruby 2.6 and Puppet 5 are EOL.
ebfc748
to
368b416
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.
looks good to me. Let's wait for feedback from @ekohl . I'm not sure if we should merge this one or wait for his PR.
@@ -27,7 +27,7 @@ Gem::Specification.new do |s| | |||
|
|||
s.add_dependency "librarianp", ">=0.6.3" | |||
s.add_dependency "rsync" | |||
s.add_dependency "puppet_forge", ">= 2.1", '< 4' | |||
s.add_dependency "puppet_forge", ">= 4.0.0" |
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.
I'd still prefer to have some semver pinning:
s.add_dependency "puppet_forge", ">= 4.0.0" | |
s.add_dependency "puppet_forge", "~> 4.0" |
Though I do wonder about keeping support for the older versions.
We ended up merging #103 which gives a merge conflict. |
And #107 is a replacement that only updates the upper bound. |
Alternative PR was merged and the next major version should include this. |
This updates the
puppet_forge
dependency to at least 4.0.0 in order to pull in Faraday 2.x and fix the deprecation warnings in #91.This also means updating the minimum supported Ruby version to 2.6 to match Faraday 2.x which may have consequences that need to be discussed.