Respect DisabledByDefault for custom cops #7013
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
XrXr
force-pushed
the
custom-cop-disabled-by-default
branch
from
7f4dea3
to
c7390bd
Compare
bbatsov
reviewed
May 13, 2019
lib/rubocop/config.rb
Outdated
| @@ -710,7 +710,10 @@ def enable_cop?(qualified_cop_name, cop_options) | |||
| end | |||
| end | |||
|
|
|||
| cop_options.fetch('Enabled', true) | |||
| cop_options.fetch('Enabled') do | |||
| disabled_by_default = for_all_cops['DisabledByDefault'] | |||
There was a problem hiding this comment.
I guess you can simplify this to simply !for_all_cops['DisabledByDefault'].
There was a problem hiding this comment.
👍 I opted for the verbose version because it looked nicer to me. Changed
|
The changes look good, but your branch has to be rebased on top of the current |
XrXr
force-pushed
the
custom-cop-disabled-by-default
branch
3 times, most recently
from
5e833cb
to
54b25c7
Compare
bbatsov
reviewed
May 14, 2019
CHANGELOG.md
Outdated
| @@ -9,7 +9,7 @@ | |||
| * Add support for subclassing using `Class.new` to `Lint/InheritException`. ([@houli][]) | |||
| * [#6779](https://github.com/rubocop-hq/rubocop/issues/6779): Add new cop `Style/NegativeUnless` that checks for unless with negative condition. ([@tejasbubane][]) | |||
| * [#6649](https://github.com/rubocop-hq/rubocop/pull/6649): Layout/AlignHash supports list of options. ([@stoivo][]) | |||
|
|
|||
| * Respect DisabledByDefault for custom cops. ([@XrXr][]) | |||
There was a problem hiding this comment.
This should go in the master section of the changelog, under Bug fixes.
XrXr
force-pushed
the
custom-cop-disabled-by-default
branch
2 times, most recently
from
a5adebd
to
84d7b5b
Compare
Before this commit, DisabledByDefault has no effect on custom cops and they are enabled when the config files contain no mention of them. This is a problematic behavior as CI servers might load multiple custom cop and run Rubocop over repos with vastly different configs. Custom cops end up enabled for repos that don't care about them, simply because they are loaded.
XrXr
force-pushed
the
custom-cop-disabled-by-default
branch
from
84d7b5b
to
7b145eb
Compare
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this commit, DisabledByDefault has no effect on
custom cops and they are enabled when the config files
contain no mention of them. This is a problematic
behavior as CI servers might load multiple custom cop
and run Rubocop over multiple repos. Custom cops end
up enabled for repos that don't care about them, even
though DisabledByDefault is set to true.
Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).and description in grammatically correct, complete sentences.
bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.