-
Notifications
You must be signed in to change notification settings - Fork 59
Upgrade RuboCop to 1.75.2 + enable new cops #984
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
Conversation
These are the fixes discovered by upgrading to RuboCop 1.69 + enabling new cops here: chef/cookstyle#984 Signed-off-by: Tim Smith <tsmith84@gmail.com>
|
Tim, I approved the other PR but I have some questions inline about this one - some of the cops appear to be duplicates of each other - the block starting around line 720 appears to be duplicated in a block at ~line 3531 or so? There's a couple of small typos that need clarifying too. |
These are the fixes discovered by upgrading to RuboCop 1.69 + enabling new cops here: chef/cookstyle#984 Signed-off-by: Tim Smith <tsmith84@gmail.com>
|
Related to: #984 |
|
4881140 to
63ad1a8
Compare
|
jaymzh
left a comment
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 looks good to me, pending #981 stuff.
|
While using the I was able to get it working temporarily by adding Does this gem need to be added as part of this project? |
|
@ahasunos rubocop-performance is already in one of the groups. Did you make sure to bundle install it first? |
|
@tas50 - Oh, I see that group :rubocop_gems do
gem 'rubocop-performance'
endHowever, since I'm using Cookstyle in my project by referencing it like this in my gem "cookstyle", git: "https://github.com/tas50/cookstyle/", branch: "tas50/bump_version"I believe this setup would install only the gems listed in the If that's the case, would it make sense to move |
63ad1a8 to
b8cefda
Compare
b8cefda to
7992f82
Compare
|
@ahasunos I removed rubocop-performance entirely so give things a try now |
This is the latest release Signed-off-by: Tim Smith <tsmith84@gmail.com>
Keep cookstyle at the earlier version so people can migrate off older chef client releases Signed-off-by: Tim Smith <tsmith84@gmail.com>
Signed-off-by: Tim Smith <tsmith84@gmail.com>
Signed-off-by: Tim Smith <tsmith84@gmail.com>
Signed-off-by: Tim Smith <tsmith84@gmail.com>
Resolves a big pile of bugs Signed-off-by: Tim Smith <tsmith84@gmail.com>
Signed-off-by: Tim Smith <tsmith84@gmail.com>
Signed-off-by: Tim Smith <tsmith84@gmail.com>
Signed-off-by: Tim Smith <tsmith84@gmail.com>
Signed-off-by: Tim Smith <tsmith84@gmail.com>
No need for this right now Signed-off-by: Tim Smith <tsmith84@gmail.com>
Signed-off-by: Tim Smith <tsmith84@gmail.com>
e95d8f5 to
611c7b9
Compare
Signed-off-by: Tim Smith <tsmith84@gmail.com>
2d7285c to
3eefdac
Compare
Signed-off-by: Tim Smith <tsmith84@gmail.com>
Signed-off-by: Tim Smith <tsmith84@gmail.com>
This is the very first Cookstyle cop and I never really touched it. It hadn't been modernized and sometime in the last 3 years RuboCop nuked the old method Signed-off-by: Tim Smith <tsmith84@gmail.com>
|
|
|
||
| ### Updated default Ruby release | ||
|
|
||
| Cookstyle now defaults to Ruby 2.6, enabling new language features, while still helping users on legacy Chef Infra Client releases to upgrade their cookbooks. |
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.
3.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.
Cookstyle previously defaulted to 2.5, which helped with really old Infra Client upgrades. You want to upgrade your cookbooks before doing the client release. By targeting an old version of Ruby by default you can resolve deprecations in your cookbooks without any modern Ruby-isms sneaking it. If you want to target the latest Infra Client release then you can bump up the target infra client and ruby versions in a .rubocop.yml file.
I did bump Chefstyle to 3.0 though since that's really just for internal use and 2.x support in Chef projects is all gone now.
| NewCops: disable | ||
| DisabledByDefault: true | ||
| TargetRubyVersion: 2.5 | ||
| TargetRubyVersion: 2.6 |
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.
wait, why is it 2.6 here and 3.0 above?
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.
Cookstyle vs. Chefstyle
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.
Isn't that just the ruby version whose "cops" are applied to your code? You're apply Ruby 2.6 syntax-checking and linting. I would expect Ruby 3.0 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.
Some cops will try to modernize your code with new methods that don't exist in old Ruby releases. If we set that to the latest and greatest Ruby releases you couldn't modernize the Chef side without introducing Ruby that old clients couldn't use. Folks can always crank that up as necessary. Ruby 2.6 is Infra Client 15.
|
Thanks for getting this merged 👍🏼 any chance we can get a release on RubyGems.org please? |



This picks up from #981 so that needs to get merged first
Chef/Style/AttributeKeys