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

feat: add whitelist of rules directives can be used to disable #208

Draft
wants to merge 2 commits into
base: feat/prevent-abusive-eslint-disables-v2
Choose a base branch
from

Conversation

ndhoule
Copy link
Contributor

@ndhoule ndhoule commented Nov 18, 2019

WIP: This is not yet a complete list. If you routinely need to disable
rules for valid reasons, add your rules here along with a justification!

This changeset adds a whitelist of list of rules that can be disabled by
eslint directives (e.g. /* eslint-disable rule-name-here */). As a
starting point, no rules can be disabled via directives. A few
exceptions have been added to the list. We should aim to keep this list
as short as possible.

Developers can still disable rules by modifying their project's ESLint
configuration, which is much easier to catch in review because new
features and bugfixes generally shouldn't require lint configuration
changes. (A nice side effect that can fall out of this is that
restricting where we can make lint configuration changes will allow us
to do things like set up policy-bot rules that require additional
approvals when a PR modifies a lint configuration to help us make sure
we upstream lint configuration changes into the common lint config.)

This changeset adds `eslint-plugin-eslint-comments` and a [set of
rules](https://github.com/mysticatea/eslint-plugin-eslint-comments/blob/d2d21f2496232248370da851d7dbd634e2a37d87/lib/configs/recommended.js)
intended to help prevent abuse of `eslint-disable` comments and to provide
feedback when `eslint-disable`s can be removed.

Closes #204.
@ndhoule ndhoule changed the title [wip] feat: add whitelist of rules directives can be used to disable feat: add whitelist of rules directives can be used to disable Nov 18, 2019
@ndhoule ndhoule requested a review from serhalp November 18, 2019 18:03
WIP: This is not yet a complete list. If you routinely need to disable
rules for valid reasons, add your rules here along with a justification!

This changeset adds a whitelist of list of rules that can be disabled by
eslint directives (e.g. `/* eslint-disable rule-name-here */`). As a
starting point, no rules can be disabled via directives. A few
exceptions have been added to the list. We should aim to keep this list
as short as possible.

Developers can still disable rules by modifying their project's ESLint
configuration, which is much easier to catch in review because new
features and bugfixes generally shouldn't require lint configuration
changes. (A nice side effect that can fall out of this is that
restricting where we can make lint configuration changes will allow us
to do things like set up `policy-bot` rules that require additional
approvals when a PR modifies a lint configuration to help us make sure
we upstream lint configuration changes into the common lint config.)
@ndhoule ndhoule force-pushed the feat/prevent-abusive-eslint-disables-v3 branch from ad78cdd to a3cc67b Compare November 18, 2019 18:21
@ndhoule ndhoule force-pushed the feat/prevent-abusive-eslint-disables-v2 branch 3 times, most recently from ad09e5a to ba64cb6 Compare November 18, 2019 21:43
@serhalp serhalp removed their request for review July 19, 2021 17:22
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.

1 participant