Skip to content

tools: add no-duplicate-requires rule #21712

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed

Conversation

devsnek
Copy link
Member

@devsnek devsnek commented Jul 8, 2018

/cc @nodejs/linting

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Jul 8, 2018
Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

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

A test for the custom rule would be good.

@devsnek devsnek force-pushed the feature/linter-no-duplicate-requires-rule branch from 38b0037 to 10d3bb2 Compare July 8, 2018 23:31
@devsnek
Copy link
Member Author

devsnek commented Jul 8, 2018

@@ -296,7 +297,7 @@ if (isMainThread) {
console.log('received:', value);
});
} else {
require('worker_threads').once('message', (value) => {
worker_threads.once('message', (value) => {
Copy link
Member

Choose a reason for hiding this comment

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

This should instead use worker_threads.parentPort (see #21486). Might be helpful to apply that first.

@Trott
Copy link
Member

Trott commented Jul 9, 2018

I wonder if this might be subsequently upstreamable to ESLint core.

@devsnek devsnek force-pushed the feature/linter-no-duplicate-requires-rule branch from 10d3bb2 to 9b92f50 Compare July 10, 2018 03:35
@devsnek
Copy link
Member Author

devsnek commented Jul 10, 2018

@Trott
Copy link
Member

Trott commented Jul 10, 2018

Looks like it needs another rebase due to a PR I just landed. Sorry.

@devsnek devsnek force-pushed the feature/linter-no-duplicate-requires-rule branch from 9b92f50 to a1e019b Compare July 10, 2018 20:50
@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 10, 2018
@addaleax
Copy link
Member

@Trott
Copy link
Member

Trott commented Jul 12, 2018

@Trott
Copy link
Member

Trott commented Jul 12, 2018

Windows fanned is "Resume Build"-resistant so here's a Rebuild instead: https://ci.nodejs.org/job/node-test-commit-windows-fanned/19249/

@Trott
Copy link
Member

Trott commented Jul 12, 2018

(All green in CI.)

@devsnek
Copy link
Member Author

devsnek commented Jul 12, 2018

landed in 8cae9b2

@devsnek devsnek closed this Jul 12, 2018
@devsnek devsnek deleted the feature/linter-no-duplicate-requires-rule branch July 12, 2018 21:11
devsnek added a commit that referenced this pull request Jul 12, 2018
PR-URL: #21712
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
targos pushed a commit that referenced this pull request Jul 14, 2018
PR-URL: #21712
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
@targos targos mentioned this pull request Jul 17, 2018
@devsnek devsnek restored the feature/linter-no-duplicate-requires-rule branch July 18, 2018 15:56
@devsnek devsnek reopened this Jul 18, 2018
@devsnek devsnek closed this Jul 18, 2018
@devsnek devsnek force-pushed the feature/linter-no-duplicate-requires-rule branch from a1e019b to e406cca Compare July 18, 2018 15:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants