Skip to content

make mry config upgrading optional #260

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

Merged
merged 2 commits into from
Oct 26, 2020

Conversation

britneywright
Copy link

@britneywright britneywright commented Oct 26, 2020

In investigating #251 Layout/FirstArgumentIndentation config options do not match rubocop mainline I learned we rely on the Mry gem to automagically update RuboCop configs between as RuboCop version change. On one hand this is nice because it brings outdated configs inline with the newer versions of RuboCop without manually checking and updating all the changing configs. Unfortunately Mry hasn't been updated since RuboCop version 0.78 and it's not negatively changing some config values that have been further modified since 0.78. Mry also never supported splitting cops or deleting cops.

In the case of the open issue here I think there was a combination of problems, one was with handling a split cop and the other is how it handles renamed cops that have been renamed again in more recent Rubocop versions. I added a config check to disable mry. By default it's enabled so users won't see any new, unexpected errors unless they choose to opt out of the mry config upgrading.

To disable rubocop plugin in .codeclimate.yml should look something like:

rubocop:
  enabled: true
  channel: rubocop-0-88
  checks:
    mry:
      disabled: true

Test Locally

  • Pull down this branch and run
IMAGE_NAME=codeclimate/codeclimate-rubocop-test make image
  • In the project of your choice replace the current rubocop plugin in .codeclimate.yml with:
  rubocop-test:
    enabled: true
    checks:
      mry:
        disabled: true
  • In the terminal of the project run:
codeclimate engines:install #removes the existing rubocop plugin
codeclimate analyze -e rubocop-test --dev #runs the CLI with the local engine

I tested this by adding the Layout/FirstArgumentIndentation cop to my rubocop yaml file with an invalid style.

Layout/FirstArgumentIndentation:
  EnforcedStyle: invalid_style

When running with Mry enabled I got an invalid EnforcedStyle error with the message:

Valid choices are: consistent, align_parentheses

This is incorrect, as Mry seems to direct the FirstArgumentIndentation to the FirstParameterIndentioncop. When I disable Mry and run the analyze again I get the expected SupportedStyles for the cop:

Valid choices are: consistent, consistent_relative_to_receiver, special_for_inner_method_call, special_for_inner_method_call_in_parentheses

Alternatives

I considered making pull requests to the Mry project rather than making this PR, but I think this is the most effective approach given how long it's been since Mry was updated. Also RuboCop just released version 1.0 so I imagine the cops will be more stable going forward and we might consider dropping Mry completely when we release new versions. If users choose to disable Mry they may need to spend some time updating older cops manually, but they'll see error messages related to those so it may end up just being tedious, rather than challenging to migrate off the Mry support.

@britneywright britneywright merged commit 219f0e2 into channel/rubocop-0-88 Oct 26, 2020
@britneywright britneywright deleted the britneywright/rubocop-0-88 branch October 28, 2020 22:54
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.

2 participants