-
Notifications
You must be signed in to change notification settings - Fork 53
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
(CAT-1688) - Pin rubocop to ~> 1.50.0
#249
Conversation
Following a recent team decision, we are implementing a Rubocop Upgrade, moving the version from 1.48.1 to 1.50.0. This should be the final version until Puppet 7 is unsupported.
Gemfile
Outdated
@@ -31,7 +31,7 @@ group :development do | |||
gem "pry", '~> 0.10', require: false | |||
gem "simplecov-console", '~> 0.5', require: false | |||
gem "puppet-debugger", '~> 1.0', require: false | |||
gem "rubocop", '= 1.48.1', require: false | |||
gem "rubocop", '= 1.50.0', require: false |
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.
within vox pupuli we use '~> 1.50.0'
. That provides all rubocop versions that still support ruby 2.6.
And Why is it limited to ruby 2.6? Is the provision gem used with the Ruby bundled by Puppet 7?
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.
for reference, that's the rubocop gems we use in our modules:
https://github.com/voxpupuli/voxpupuli-test/blob/master/voxpupuli-test.gemspec#L33-L38
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.
Thanks for reminding me of the ~>, I meant to address that but got distracted by another issue.
As for the ruby 2.6 limitation, it comes from puppetserver. While we officially support Ruby 2.7+, puppetserver makes use of Ruby 2.6 in some instances. Therefore, we need to make sure we are compatible with that as well.
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.
But is that gem used by the puppetserver jruby? Otherwise the limitation doesn't matter?
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 can't answer out of the top of my head. I remember this being a topic of discussion about a year ago when we were working on the Puppet 8 update for the modules. I will ask the team to see if anyone remember the exact reasoning behind the 2.6 pin.
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.
Coming back to this topic, I think our consensus is that we established 2.6 as our TargetRubyVersion just to be safe against potential compatibility problems back when we were starting our Puppet 8 upgrade. Currently we don't find it a priority to revert this decision as we don't see it hindering our tools health. There is other work regarding Rubocop and proper syntaxing in our tools that takes preference at the moment.
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.
But wasn't that decision only for modules?
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.
https://github.com/puppetlabs/rspec-puppet/blob/main/rspec-puppet.gemspec for example requires Ruby 2.7
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.
Back then, the DevX and Modules teams were one and the same. Whatever decisions we took at that point, they applied (to the best of knowledge) to both sides (for the sake of consistency amongst other reasons).
Thats the reason why this tool has a TargetRubyVersion set to 2.6 dating back 9 months.
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.
Yes, I'm aware there are some inconsistencies here and there. These are due to the fact that, being more limited in our manpower (a year ago), we had to put a higher priority on maintaining and keeping consistencies within the Modules side. As a result, some tools took a much longer time to be updated. And as such, some rules changed and/or some exceptions were made.
ee55a7f
to
f0040a2
Compare
9930f92
to
265ce67
Compare
265ce67
to
84999d1
Compare
~> 1.50.0
Following a recent team decision, we are implementing a Rubocop Upgrade, moving the version from 1.48.1 to 1.50.0. This should be the final version until Puppet 7 is unsupported.