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

Replace DisabledByDefault with disabling each cop #26

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shima-zu
Copy link

@shima-zu shima-zu commented Nov 20, 2024

Summary

When DisabledByDefault is true, all cops are disabled in the default configuration. This means that all extension cops are disabled by default as well.

The extensions are outside the scope of omakase, and user may not realize that it has been disabled.

Workaround

The following is workaround, but I don't want to write them in my configuration.

inherit_gem:
  rubocop-rails-omakase: rubocop.yml

inherit_from:
  - .rubocop_todo.yml

require:
  - rubocop-rspec
  - rubocop-rspec_rails

RSpec:
  Enabled: true
...

RSpecRails:
  Enabled: true
...

@koic
Copy link
Contributor

koic commented Nov 22, 2024

IMHO, this seems like a reasonable change in the sense that it only affects the minimal set of cops targeted by rubocop-rails-omakase. From the perspective of dependencies, how 3rd-party cops that are not detected by rubocop-rails-omakase behave can reasonably be considered outside the scope of its influence.

It also appears to provide a solution to the issue raised in #14. While listing departments may introduce some redundancy, to my knowledge, there is currently no better method for specifying them.

Exclude:
- "test/**/*"
Rails:
Enabled: false
Copy link
Contributor

Choose a reason for hiding this comment

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

As with cop names, these department names should also be listed in ASCII order.

Copy link
Author

@shima-zu shima-zu Nov 25, 2024

Choose a reason for hiding this comment

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

Fixed 5a5b628

@shima-zu shima-zu requested a review from koic November 25, 2024 02:43
@shima-zu shima-zu force-pushed the replace-DisabledByDefault-true branch from b8fb7b1 to 5a5b628 Compare November 25, 2024 02:50
@koic
Copy link
Contributor

koic commented Nov 25, 2024

I'm not sure if this proposal will be accepted, but for now, squashing the commits into one would be appropriate.

When `DisabledByDefault` is `true`, all cops in the default configuration.
This means that all extension cops are disabled by default.

The extensions are outside the scope of omakase, and you may not realize that it has been disabled.
@shima-zu shima-zu force-pushed the replace-DisabledByDefault-true branch from 5a5b628 to 1210690 Compare November 25, 2024 11:09
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