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

implement Style/DisableCopsWithinSourceCodeDirective cop #7384

Merged

Conversation

egze
Copy link
Contributor

@egze egze commented Sep 27, 2019

This cop forbids enabling or disabling cops within source code with comments like:

# rubocop:disable Style/EmptyBlockParameter

The motivation behind it is to make sure developers fix the issues reported by rubocop and not quickly add comments to ignore them.


Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

@egze egze force-pushed the disable_cops_within_source_code_directive branch 6 times, most recently from 0cf2768 to 1c85f00 Compare September 27, 2019 15:46
@bbatsov
Copy link
Collaborator

bbatsov commented Mar 21, 2020

I like the spirit of the change, but I'm not sure if this should be a cop or some global config in RuboCop. @jonas054 What do you think on the subject?

@jonas054
Copy link
Collaborator

I love the idea! In fact, I argued rather stubbornly for implementing this kind of functionality in a cop back in #900, but I was overruled and and the --format disabled CLI option was added instead. This option doesn't currently work (I just discovered), and hasn't worked for a long time. That means that people aren't using it.

So, I like the cop and the changes look good (had some minor comments), and I would like to see the DisabledLinesFormatter gone as well.

Copy link
Collaborator

@jonas054 jonas054 left a comment

Choose a reason for hiding this comment

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

Looks good overall.

@egze egze force-pushed the disable_cops_within_source_code_directive branch 5 times, most recently from 21f3e13 to 40a4e7d Compare March 29, 2020 17:39
@egze
Copy link
Contributor Author

egze commented Mar 29, 2020

So, I like the cop and the changes look good (had some minor comments), and I would like to see the DisabledLinesFormatter gone as well.

Removed DisabledLinesFormatter as well.

@egze egze force-pushed the disable_cops_within_source_code_directive branch 2 times, most recently from b15611c to bd3d7e9 Compare March 29, 2020 17:48
@egze egze requested a review from jonas054 March 29, 2020 18:02
@bbatsov
Copy link
Collaborator

bbatsov commented Mar 29, 2020

I love the idea! In fact, I argued rather stubbornly for implementing this kind of functionality in a cop back in #900, but I was overruled and and the --format disabled CLI option was added instead. This option doesn't currently work (I just discovered), and hasn't worked for a long time. That means that people aren't using it.

Amazing memory! I had totally forgotten about this!

So, I like the cop and the changes look good (had some minor comments), and I would like to see the DisabledLinesFormatter gone as well.

Agreed!

Copy link
Collaborator

@jonas054 jonas054 left a comment

Choose a reason for hiding this comment

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

Looks good apart from one issue in the changelog.

CHANGELOG.md Outdated Show resolved Hide resolved
@egze egze requested a review from jonas054 March 30, 2020 17:59
@egze egze force-pushed the disable_cops_within_source_code_directive branch from 6394a57 to f94d3e0 Compare April 1, 2020 06:42
@egze egze force-pushed the disable_cops_within_source_code_directive branch from f94d3e0 to 8167c38 Compare April 1, 2020 06:43
Copy link
Collaborator

@jonas054 jonas054 left a comment

Choose a reason for hiding this comment

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

👍

@jonas054 jonas054 merged commit 29b16fd into rubocop:master Apr 3, 2020
koic added a commit to koic/rubocop that referenced this pull request Apr 13, 2020
Follow rubocop#7384, rubocop#7857, and rubocop#7869.

This PR fixes the next release version with 0.82 because RuboCop 0.81
has been released. This version (0.82) is based on rubocop#7851.
@koic koic mentioned this pull request Apr 13, 2020
8 tasks
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.

None yet

3 participants