Chore: enable a modified version of multiline-comment-style rule on codebase #9452
Conversation
|
LGTM |
tests/bin/eslint.js
Outdated
| it("has exit code 1 if a linting error is reported", () => | ||
| assertExitCode(runESLint(["bin/eslint.js", "--env", "es6", "--no-eslintrc", "--rule", "semi: [2, never]"]), 1)); | ||
| it("has exit code 1 if a syntax error is thrown", () => | ||
| assertExitCode(runESLint(["README.md", "--rulesdir=tools/internal-rules"]), 1)); |
There was a problem hiding this comment.
Why did you change these lines?
There was a problem hiding this comment.
These test cases rely on ESLint linting itself. I added --rulesdir=tools/internal-rules to the second test case to ensure that the config file is valid for that run. (This is now necessary because the config file for Makefile.js involves an internal rule.) The goal is to ensure that the exit code is 1 as a result of the syntax error, not as a result of the missing rule.
|
it has a conflict. @not-an-aardvark |
not-an-aardvark
changed the title
not-an-aardvark
changed the title
not-an-aardvark
changed the title
not-an-aardvark
changed the title
not-an-aardvark
force-pushed
the
enable-multiline-comment-style
branch
from
1f23051
to
d7f7048
Compare
This makes the following changes:
* Creates a project-specific `internal-multiline-comment-style` rule,
which is the same as the built-in `multiline-comment-style` rule
except that it ignores the banner comments that we use at the top of
files.
* Enables the `internal-multiline-comment-style` rule on the ESLint
codebase
* Updates a few tests (namely: `tests/lib/cli-engine.js` and
`tests/bin/eslint.js`) that rely on linting the CWD, to account for
the fact that the config in the CWD now has a project-specific rule.
not-an-aardvark
force-pushed
the
enable-multiline-comment-style
branch
from
d7f7048
to
01224ff
Compare
|
Fixed the conflict. |
| * | ||
| * Shorthand for performance. | ||
| * | ||
| */ |
There was a problem hiding this comment.
is the L49/L51 intentional？
There was a problem hiding this comment.
It was created by the autofixer. The original comment also contained blank lines, so I think it's fine that the updated comment has them as well.
| * | ||
| * Shorthand for performance. | ||
| * | ||
| */ |
There was a problem hiding this comment.
is the L49/L51 intentional？
| //------------------------------------------------------------------------------ | ||
|
|
||
| // The `no-invalid-meta` internal rule has a false positive here. | ||
| // eslint-disable-next-line rulesdir/no-invalid-meta |
There was a problem hiding this comment.
unrelated: no-invalid-meta seems ideal to be a rule in eslint-plugin-eslint-plugin, so that other plugin authors can use easily.
eslint-deprecated
bot
added
the
archived due to age
What is the purpose of this pull request? (put an "X" next to item)
[x] Other, please explain:
What changes did you make? (Give an overview)
This makes the following changes:
internal-multiline-comment-stylerule, which is the same as the built-inmultiline-comment-stylerule except that it ignores the banner comments that we use at the top of files. This uses eslint-rule-composer to build off of the existing rule, rather than copying it and making changes.internal-multiline-comment-stylerule on the ESLint codebasetests/lib/cli-engine.jsandtests/bin/eslint.js) that rely on linting the CWD, to account for the fact that the config in the CWD now has a project-specific rule.Is there anything you'd like reviewers to focus on?
Nothing in particular