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
[Fix #9519] Disable all cop department with directive comment #9626
[Fix #9519] Disable all cop department with directive comment #9626
Conversation
200aaf8
to
dc953c6
Compare
dc953c6
to
fb288aa
Compare
lib/rubocop/directive_comment.rb
Outdated
@@ -6,7 +6,9 @@ module RuboCop | |||
# cops it contains. | |||
class DirectiveComment | |||
# @api private | |||
REDUNDANT_COP = 'Lint/RedundantCopDisableDirective' | |||
REDUNDANT_COP_DEPARTMENT = 'Lint' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really think we should name this REDUNDANT_DIRECTIVE_COP...
, otherwise the name is both funny and confusing. :-) I know that's not related to the work you've done, but I was just amused by reading the diff - the name implies it's some redandant cop. :D
lib/rubocop/directive_comment.rb
Outdated
exclude_redundant_cop(Cop::Registry.global.names) | ||
end | ||
|
||
def all_cop_names_for_department(department) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd name this cop_names_for_department
.
lib/rubocop/directive_comment.rb
Outdated
department == REDUNDANT_COP_DEPARTMENT ? exclude_redundant_cop(names) : names | ||
end | ||
|
||
def exclude_redundant_cop(cops) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's name this exclude_redundant_directive_cop
or something like this.
it { is_expected.to eq %w[Foo/Bar Foo/Baz Baz/Cop] } | ||
end | ||
|
||
context 'when redundant cop department specified' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redundant directive cop...
0b3ff68
to
6258ce9
Compare
@dvandersluis @bbatsov |
cff0c7f
to
876091a
Compare
4e53676
to
5af6859
Compare
e1d1bc9
to
573ce77
Compare
@dvandersluis @bbatsov lets discuss this PR :-) |
2b70323
to
a770934
Compare
@dvandersluis @bbatsov could you take a look? |
a770934
to
4536883
Compare
Sorry for the slow feedback - I had a couple of pretty busy weeks. The diff is huge, so I just skimmed over it and things are looking good to me overall. I'll try to review this more carefully in the next couple of days and I'm inviting everyone from @rubocop/rubocop-core to share their feedback as well. |
@@ -503,5 +503,92 @@ def bar | |||
end | |||
end | |||
end | |||
|
|||
context 'with all department disabling' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"all department disabling" sounds wrong to me. Probably you meant to write "with a disabled department" or something like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced
@@ -654,6 +654,15 @@ file by adding a comment such as | |||
# rubocop:enable Layout/LineLength, Style/StringLiterals | |||
---- | |||
|
|||
You can also disable entire departments by giving a department name in the comment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add another example showing that you can also combine in the comments department references with individual cops.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Combined with individual cop
2b6a999
to
edc5717
Compare
Co-authored-by: Daniel Vandersluis <daniel.vandersluis@gmail.com>
edc5717
to
5a7b8f5
Compare
New feature for request #9519
Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.md
if the new code introduces user-observable changes. See changelog entry format for details.