rubocop-rails-omakase icon indicating copy to clipboard operation
rubocop-rails-omakase copied to clipboard

Remove duplicate settings already configured by default

Open r7kamura opened this issue 1 year ago • 3 comments

Summary

Some settings are already set by default on the inherited gems side.

Why not remove duplicate settings in order to draw the reader's attention to the more essential configuration?

Details

Layout/EndAlignment

AutoCorrect: true is configured by default.

Layout/IndentationConsistency

Enabled: false is configured by default.

Also, do not add any settings for the disabled cops.

Layout/IndentationWidth

Enabled: false is configured by default.

Layout/SpaceInsideArrayLiteralBrackets

EnforcedStyleForEmptyBrackets: no_space is configured by default.

  • https://github.com/rubocop/rubocop/blob/v1.59.0/config/default.yml#L1429

Layout/SpaceInsideHashLiteralBraces

EnforcedStyleForEmptyBraces: no_space is configured by default.

  • https://github.com/rubocop/rubocop/blob/v1.59.0/config/default.yml#L1469

Layout/SpaceInsidePercentLiteralDelimiters

Enabled: false is configured by default.

Rails/AssertNot

Include: ["**/test/**/*"] is configured by default.

  • https://github.com/rubocop/rubocop-rails/blob/v2.23.1/config/default.yml#L226-L227

Also, do not add any settings for the disabled cops.

Rails/RefuteMethods

Include: ["**/test/**/*"] is configured by default.

  • https://github.com/rubocop/rubocop-rails/blob/v2.23.1/config/default.yml#L865-L866

Also, do not add any settings for the disabled cops.

Style/AndOr

Enabled: false is configured by default.

Style/PercentLiteralDelimiters

PreferredDelimiters: ... is configured by default with the same settings.

  • https://github.com/rubocop/rubocop/blob/v1.59.0/config/default.yml#L4780-L4786

Style/RedundantPercentQ

Enabled: false is configured by default.

r7kamura avatar Jan 04 '24 02:01 r7kamura

do not add any settings for the disabled cops.

To be honest, I am not exactly sure if this decision is correct. This is because, for example, rubocop-minitest and rubocop-rails are actually disabled with DisabledByDefault: true even though they are required, so they are not effective as they are.

The fact that they are required means that they are intended to be enabled on the user side, right? So, the act of disabling it but adding the setting may be meaningful.

r7kamura avatar Jan 04 '24 02:01 r7kamura

I added some more detail in https://github.com/rails/rubocop-rails-omakase/issues/14. My recommendation would be to remove DisabledByDefault: true and instead maintain these explicit enables/disables.

bensheldon avatar Jan 06 '24 16:01 bensheldon

Maybe the idea of these explicit Enabled: false was to guarantee that they will still be disabled in configurations that inherit rubocop-rails-omakase but have DisabledByDefault: false. Just my suggestion.

igor-alexandrov avatar Jan 10 '24 07:01 igor-alexandrov

Maybe the idea of these explicit Enabled: false was to guarantee that they will still be disabled in configurations that inherit rubocop-rails-omakase but have DisabledByDefault: false. Just my suggestion.

Yes. They'll also override preceding config.

The key thing is that presence of the cops communicates intent, regardless of default.

jeremy avatar Nov 03 '24 20:11 jeremy