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

Run 2.6 rubocop #162

Closed
wants to merge 8 commits into from

Conversation

@zachsabin
Copy link
Collaborator

zachsabin commented Nov 8, 2019

This PR splits the rubocop configuration based on which version of rubocop is being used. It uses rubocop version 0.76 for Ruby 2.6 and rubocop version 0.58 for <2.6. It sets the version of rubocop based on which version of ruby is being used. Important commits are:

  1. 849605d -- sets up the dependency versioning based on which version of ruby is being used (the yml config changes there eventually end up in the 0.76 folder, but they're not super important)
  2. e1e86de -- sets up the configuration such that it pulls yml files from the relevant folder for the version of rubocop that it's using.

Next steps if this approach seems reasonable:

  1. Running in 2.6 there are a lot of warnings about unrecognized cops. I'll have to go through them at some point and either remove or update them.
  2. Ensure that we have coverage of all the styles we want on 0.76
  3. Try to reconcile the freeze rule on version.rb -- 0.76 complains about the freeze but 0.58 requires it. The redundant freeze rule name changed from 0.58 to 0.76, so if you disable it with a comment then 0.58 complains about a redundant disabling. But the name of the rule that checks for redundant disables also changed! So there is no way to get that line to pass without disabling the redundant freeze rule in 0.76 at the moment.
zachary_sabin added 2 commits Nov 8, 2019
zachary_sabin
@zachsabin zachsabin force-pushed the zachsabin:zs--2.6-compatible branch from c20ca68 to 6690eb8 Nov 8, 2019
@@ -227,7 +227,7 @@ Layout/FirstMethodParameterLineBreak:
Enabled: false

# Supports --auto-correct
Layout/FirstParameterIndentation:
Layout/IndentFirstArgument:

This comment has been minimized.

Copy link
@ljharb

ljharb Nov 8, 2019

Contributor

changing these rule names seems like a breaking change. why?

This comment has been minimized.

Copy link
@zachsabin

zachsabin Nov 8, 2019

Author Collaborator

I copied the new yml configs from #150. If the older ones are forwards compatible, then there's no reason to do that (should have checked!)

This comment has been minimized.

Copy link
@zachsabin

zachsabin Nov 8, 2019

Author Collaborator

They are not forwards compatible:

Error: The `Layout/FirstParameterIndentation` cop has been renamed to `Layout/IndentFirstArgument`.
(obsolete configuration found in config/0.76/rubocop-layout.yml, please update it)
The `Layout/IndentArray` cop has been renamed to `Layout/IndentFirstArrayElement`.
(obsolete configuration found in config/0.76/rubocop-layout.yml, please update it)
The `Layout/IndentHash` cop has been renamed to `Layout/IndentFirstHashElement`.
(obsolete configuration found in config/0.76/rubocop-layout.yml, please update it)

This comment has been minimized.

Copy link
@zachsabin

zachsabin Nov 8, 2019

Author Collaborator

These two commits make it backwards compatible by splitting what config is used based on ruby version:
aaf5dd8
1c0591d

That said it's a bit of a hack

@ljharb

This comment has been minimized.

Copy link
Contributor

ljharb commented Nov 8, 2019

That means rubocop 2.6 is a semver-major, and should have been 3.0

zachary_sabin added 2 commits Nov 8, 2019
@zachsabin zachsabin force-pushed the zachsabin:zs--2.6-compatible branch from 0c0ffae to 2222601 Nov 8, 2019
@zachsabin

This comment has been minimized.

Copy link
Collaborator Author

zachsabin commented Nov 8, 2019

Well it's really rubocop 0.58 -> 0.76 but I agree! Unfortunately if we want to run stuff with ruby 2.6 then we have to get to a higher version than 0.58.

zachary_sabin
@zachsabin

This comment has been minimized.

Copy link
Collaborator Author

zachsabin commented Nov 8, 2019

Woohoo CI is passing! I will write a longer description of what I did in the PR description. I'm not 100% sure if it's something we want to do but I would really like to be able to run rubocop with 2.6.

@@ -1,6 +1,5 @@
inherit_from:
- .rubocop_airbnb.yml
- ./config/default.yml

This comment has been minimized.

Copy link
@zachsabin

zachsabin Nov 8, 2019

Author Collaborator

This did not contribute any configs that weren't already being pulled in by our injection code. I checked via bundle exec rubocop --config .rubocop.yml --show-cops and the same number of cops were being used with and without it.

zachary_sabin added 3 commits Nov 8, 2019
zachary_sabin
zachary_sabin
zachary_sabin
@zachsabin

This comment has been minimized.

Copy link
Collaborator Author

zachsabin commented Nov 8, 2019

Closing as I think we're just going to drop support for ruby versions <2.3, rendering this needlessly complex.

@zachsabin zachsabin closed this Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.