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

Remove redundant cops #88

Merged
merged 5 commits into from
Jun 3, 2020
Merged

Remove redundant cops #88

merged 5 commits into from
Jun 3, 2020

Conversation

kevindew
Copy link
Member

@kevindew kevindew commented Jun 2, 2020

This removes a number of cops that aren't necessary as they don't deviate from rubocops defaults. I tried this out on Content Publisher and Whitehall and saw no new issues raised by this.

I also wondered if we should remove

# Part of the orignal GDS styleguide
# "Prefer %w to the literal array syntax when you need
# an array of strings."
# https://github.com/alphagov/styleguides/blob/6395a10d41c3938f4c147cda443fd83f854c3e7a/ruby.md#collections
Style/WordArray:
MinSize: 0

Since this only differs from the rubocop defaults by applying it at lengths < 1. Arguably the most surprising occurrence of our cop is when it tells you to change ["item"] into %w[item]`. I decided against doing this on this PR as it would have an affect unlike the other ones.

Copy link
Contributor

@benthorner benthorner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one! Great to get rid of some more stuff. I think when I did my original audit, I only checked for ones that were disabled by default.

Regarding Style/WordArray, I can see a couple of reasons to keep it as-is:

  • It's applied consistently, so we don't have to remember essentially two different rules
  • It's consistent with other config that disables heuristic (count, length, etc.) behaviour

# https://github.com/alphagov/styleguides/blob/6395a10d41c3938f4c147cda443fd83f854c3e7a/ruby.md#collections
Style/HashSyntax:
Exclude:
- 'db/schema.rb'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevindew kevindew merged commit 7cffceb into master Jun 3, 2020
@kevindew kevindew deleted the remove-redundant branch June 3, 2020 08:38
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.

None yet

2 participants