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

Rubocop: Bump Ruby version to 2.5 and define new cops #14639

Closed
wants to merge 1 commit into from

Conversation

bcoles
Copy link
Contributor

@bcoles bcoles commented Jan 21, 2021

Resolves #14465

New Cops

I have enabled all new cops to watch the world burn.

In seriousness, I had a quick look through the new cops and they seem reasonable at first glance.

All new cops have been marked as disabled with a comment to review these in the future.

Ruby Version

The Ruby version has been bumped from 2.4 to 2.7.

Ruby 2.4 is end of life. Ruby 2.5 is end of life in a couple of months.

The .ruby-version file specifies Ruby version 2.7.2.

It makes sense that we use 2.7 for Rubocop.

@jmartin-tech
Copy link
Contributor

jmartin-tech commented Jan 25, 2021

Would it be better to set the rules to match the minimum currently tested version in automation? At this time that is Ruby 2.5. Updating rules to match what will be evaluated by specs seems like a reasonable step forward.

I support the idea of dropping supported for versions when official Ruby support ends.

@bcoles
Copy link
Contributor Author

bcoles commented Jan 25, 2021

Would it be better to set the rules to match the minimum currently tested version in automation? At this time that is Ruby 2.5. Updating rules to match what will be evaluated by specs seems like a reasonable step forward.

I support the idea of dropping supported for versions when official Ruby support ends.

My goal is to bump the Ruby version and explicitly define the undefined cops. I'll take the path of least resistance to get this merged.

Ruby 2.5 is already in maintenance pending EOL.

.rubocop.yml currently specifies version 2.4. Ruby 2.4 support has officially ended almost a year ago.

TargetRubyVersion: 2.4

Metasploit lists version 2.7.2 in .ruby-version:

Metasploit lists version >= 2.5 in metasploit-framework.gemspec:

spec.required_ruby_version = '>= 2.5'

At the very minimum the rubocop ruby version should by bumped to version 2.5. However, the downloads page on Ruby website states the 2.5 branch is "in security maintenance phase (will EOL soon!)".

Presuming they stick to the same EOL lifetime as previous releases branches, 2.5 will be EOL on March 31st, which is 9 weeks away.

I understand the potential need to support Ruby 2.6, in which case it may make more sense to bump the version to 2.6 rather than 2.7.

Likewise, in the interest of getting this merged and moving on, we can set all the new cops to disabled with a comment for each to be reviewed in the future

@bcoles bcoles changed the title Rubocop: Bump Ruby version to 2.7 and add new cops Rubocop: Bump Ruby version to 2.6 and define new cops Jan 25, 2021
@bcoles
Copy link
Contributor Author

bcoles commented Jan 25, 2021

Bumped Ruby version to 2.6. Let me know if this is a problem and I'll change it to 2.5.

@adfoster-r7 adfoster-r7 self-assigned this Jan 27, 2021
@bcoles bcoles changed the title Rubocop: Bump Ruby version to 2.6 and define new cops Rubocop: Bump Ruby version to 2.5 and define new cops Feb 6, 2021
@bcoles
Copy link
Contributor Author

bcoles commented Feb 6, 2021

Bumped Ruby version from 2.4 to 2.5 (rather than 2.6) in the interest of getting this landed.

Syntax changes in 2.5 are as follows:

Most of the changes in 2.5 are acceptable. The nil safe operator &. is the most contentious. There is no consensus as yet as to whether this should be used in Framework.

@bcoles bcoles closed this Feb 8, 2021
@bcoles bcoles deleted the rubocop branch February 8, 2021 04:56
@adfoster-r7
Copy link
Contributor

Most of the changes in 2.5 are acceptable. The nil safe operator &. is the most contentious.

We had to disable the safe navigation rule in the end as it caused problems with modules: #13347

That's not to say it shouldn't be used, just that enforcing it with rubocop is problematic

@adfoster-r7
Copy link
Contributor

Hoping to look at this soon, been stuck looking at the Ruby 3.0 upgrade work. I have a branch waiting that aims to enforce rubocop within msftidy, and I've also been looking at how the existing/new rules would have impacted recently landed modules to see if there are any changes to be made 👍

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.

The following cops were added to RuboCop, but are not configured
3 participants