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

config/default: Remove some redundant Enabled: true for cops #128

Merged
merged 2 commits into from
Oct 25, 2022

Conversation

issyl0
Copy link
Member

@issyl0 issyl0 commented Oct 21, 2022

  • For cops with a styleguide link, where there's evidence that we want the rule to apply, we don't need to also explicitly declare Enabled: true for each of them. Since we removed DisabledByDefault, the enablements here for cops that are enabled by default in RuboCop itself* are redundant.
  • I could have gone further and removed the rules that RuboCop has enabled by default that didn't have a styleguide link, but I feel like we need some evidence for why a rule is in place first for when people come looking at this gem's config to figure out why new rules are suddenly failing or not.

- For cops with a styleguide link, where there's evidence that we want
  the rule to apply, we don't need to also explicitly declare
  `Enabled: true` for each of them. Since we removed `DisabledByDefault`,
  the enablements here for cops that are *enabled by default in RuboCop
  itself** are redundant.
- I could have gone further and removed the rules that RuboCop has
  enabled by default that _didn't_ have a styleguide link, but I feel
  like we need some evidence for why a rule is in place first for when
  people come looking at this gem's config to figure out why new rules
  are suddenly failing or not.
@issyl0 issyl0 merged commit c79e440 into main Oct 25, 2022
@issyl0 issyl0 deleted the enable-some-more-cops branch October 25, 2022 17:39
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