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

"Layout/AlignHash" Cop renamed in v0.77 #7

Closed
dylnclrk opened this issue Dec 2, 2019 · 4 comments
Closed

"Layout/AlignHash" Cop renamed in v0.77 #7

dylnclrk opened this issue Dec 2, 2019 · 4 comments

Comments

@dylnclrk
Copy link
Member

dylnclrk commented Dec 2, 2019

The Rubocop 0.77 release "standardized" some of the cop names. Unfortunately, our conventions disable one of the renamed cops (Layout/AlignHash).

Layout/AlignHash:
Enabled: false

Users of our conventions will see an error when they upgrade to v0.77:

Error: The `Layout/AlignHash` cop has been renamed to `Layout/HashAlignment`.
(obsolete configuration found in .rubocop-https---raw-githubusercontent-com-carbonfive-c5-conventions-master-rubocop-rubocop-yml, please update it)

I think we should update the conventions file. However, this may break things for users of Rubocop < v0.77.

Is there a good way to deal with this?

@dylnclrk dylnclrk changed the title Update name of Layout/AlignHash to Layout/HashAlignment "Layout/AlignHash" Cop renamed in v0.77 Dec 2, 2019
@mattbrictson
Copy link
Contributor

I think it is OK to break backwards compatibility; we have a precedent for this. We added rubocop-performance in 2ecae4a which would break projects downstream until they added a rubocop-performance to their Gemfiles.

In terms of better ways to deal with this, I believe that projects can reference the conventions file by git SHA (e.g. https://raw.githubusercontent.com/carbonfive/c5-conventions/2ecae4ad486cf2de4d1b5d3113f2be12bf406123/rubocop/rubocop.yml) so that they don't track the latest changes of the master branch.

There is also a way to distribute rubocop conventions as a gem: https://docs.rubocop.org/en/stable/configuration/#inheriting-configuration-from-a-remote-url. That would be more work in terms of packaging but it would then allow each project to pin to a version of the conventions via a gem dependency.

Personally I think there are two slightly different use cases that might warrant different solutions.

  1. For internal C5 projects, we want all of our code bases aligned on the same conventions. In this case it makes sense for all our internal projects to track the master branch.
  2. For client projects, we want to use the C5 rubocop conventions as a starting point, but then each team is likely to evolve their own set of preferences depending on the project and the client. In that case I would recommend copying the contents of the conventions .rubocop.yml as a starting point but not maintain an actual link to the C5 conventions repo.

dylnclrk added a commit that referenced this issue Dec 2, 2019
Layout/Align hash was renamed (#7), change the config file accordingly.
@dylnclrk dylnclrk mentioned this issue Dec 2, 2019
@dylnclrk
Copy link
Member Author

dylnclrk commented Dec 2, 2019

@mattbrictson that makes sense. I was thinking about what happened with rubocop-performance when I opened this issue. I'll open a PR to keep us up to date.

Also, this is a good suggestion:

For client projects, we want to use the C5 rubocop conventions as a starting point, but then each team is likely to evolve their own set of preferences depending on the project and the client. In that case I would recommend copying the contents of the conventions .rubocop.yml as a starting point but not maintain an actual link to the C5 conventions repo.

@dylnclrk
Copy link
Member Author

dylnclrk commented Dec 2, 2019

Oops, I think there's more:

Layout/IndentFirstArrayElement cop has been renamed to Layout/FirstArrayElementIndentation.

Will fix the others!

dylnclrk added a commit that referenced this issue Dec 2, 2019
Layout/AlignHash, and other cops were renamed in Rubocop 0.77
(see #7). Change the config accordingly.
@dylnclrk
Copy link
Member Author

dylnclrk commented Dec 2, 2019

Closed in #8

@dylnclrk dylnclrk closed this as completed Dec 2, 2019
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

No branches or pull requests

2 participants