src: fix unused namespace member in node_util by puzpuzpuz · Pull Request #34565 · nodejs/node

puzpuzpuz · 2020-07-30T17:33:43Z

make -j4 test currently fails with the following message:

Running C++ linter...
File "src/node_util.cc" does not use "MaybeLocal"
Makefile:1337: recipe for target 'tools/.cpplintstamp' failed
make[2]: *** [tools/.cpplintstamp] Error 1
Makefile:1378: recipe for target 'lint' failed
make[1]: *** [lint] Error 2

Checklist

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

richardlau · 2020-07-30T18:18:41Z

It's puzzling our CI's (neither Actions nor Jenkins) failed to pick that up.

puzpuzpuz · 2020-07-30T19:33:48Z

@richardlau yeah, that's surprising for me as well.

addaleax · 2020-07-30T21:30:50Z

Even more surprising, locally:

$ python3.6 tools/checkimports.py 
File "src/node_util.cc" does not use "MaybeLocal"
$ python2.7 tools/checkimports.py 
File "src/node_util.cc" does not use "MaybeLocal"
File "src/node_report.cc" does not use "Number"
File "src/node_report.cc" does not use "StackTrace"
using statements aren't sorted in 'src/node_report.cc'.
	Line 51: Actual: v8::Value, Expected: v8::V8
	Line 52: Actual: v8::V8, Expected: v8::Value

addaleax · 2020-07-30T23:39:25Z

Also, let’s fast-track this. We can take care of the non-surfacing weirdnesses after that. :)

richardlau · 2020-07-30T23:40:52Z

Also, let’s fast-track this. We can take care of the non-surfacing weirdnesses after that. :)

👍 to both statements.

addaleax · 2020-07-31T12:36:13Z

Landed in 1e47051

PR-URL: #34565 Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Makefile assumes that it can pass a list of files to the import checker, whereas the import checker expects a single argument that is interpreted as a blob. Fix that mismatch by accepting multiple arguments in the import checker. Refs: nodejs#34565

PR-URL: #34565 Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Fix linter failures when running the linter on all source files. PR-URL: #34582 Refs: #34565 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Makefile assumes that it can pass a list of files to the import checker, whereas the import checker expects a single argument that is interpreted as a blob. Fix that mismatch by accepting multiple arguments in the import checker. Refs: #34565 PR-URL: #34582 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Fix linter failures when running the linter on all source files. PR-URL: #34582 Refs: #34565 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Makefile assumes that it can pass a list of files to the import checker, whereas the import checker expects a single argument that is interpreted as a blob. Fix that mismatch by accepting multiple arguments in the import checker. Refs: #34565 PR-URL: #34582 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Fix linter failures when running the linter on all source files. PR-URL: #34582 Refs: #34565 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Makefile assumes that it can pass a list of files to the import checker, whereas the import checker expects a single argument that is interpreted as a blob. Fix that mismatch by accepting multiple arguments in the import checker. Refs: #34565 PR-URL: #34582 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

PR-URL: #34565 Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

src: fix unused namespace member in node_util

1591ad4

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. util Issues and PRs related to the built-in util module. labels Jul 30, 2020

himself65 approved these changes Jul 30, 2020

View reviewed changes

richardlau approved these changes Jul 30, 2020

View reviewed changes

jasnell approved these changes Jul 30, 2020

View reviewed changes

addaleax added the fast-track PRs that do not need to wait for 48 hours to land. label Jul 30, 2020

addaleax closed this Jul 31, 2020

addaleax pushed a commit that referenced this pull request Jul 31, 2020

src: fix unused namespace member in node_util

1e47051

PR-URL: #34565 Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

puzpuzpuz deleted the cleanup/remove-unused-import-from-node-util branch July 31, 2020 12:38

addaleax mentioned this pull request Jul 31, 2020

tools: fix C++ import checker argument expansion #34582

Closed

3 tasks

codebytere pushed a commit that referenced this pull request Aug 5, 2020

src: fix unused namespace member in node_util

c1abc8d

PR-URL: #34565 Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

codebytere mentioned this pull request Aug 10, 2020

v14.8.0 proposal #34704

Merged

addaleax pushed a commit that referenced this pull request Sep 22, 2020

src: fix unused namespace member in node_util

761c6f4

PR-URL: #34565 Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

addaleax pushed a commit that referenced this pull request Sep 22, 2020

src: fix unused namespace member in node_util

c0a961e

PR-URL: #34565 Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

codebytere mentioned this pull request Sep 28, 2020

v12.19.0 proposal #35401

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

src: fix unused namespace member in node_util#34565

src: fix unused namespace member in node_util#34565
puzpuzpuz wants to merge 1 commit intonodejs:masterfrom
puzpuzpuz:cleanup/remove-unused-import-from-node-util

puzpuzpuz commented Jul 30, 2020

Uh oh!

richardlau commented Jul 30, 2020 •

edited

Loading

Uh oh!

puzpuzpuz commented Jul 30, 2020

Uh oh!

addaleax commented Jul 30, 2020

Uh oh!

addaleax commented Jul 30, 2020

Uh oh!

richardlau commented Jul 30, 2020

Uh oh!

addaleax commented Jul 31, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

puzpuzpuz commented Jul 30, 2020

Checklist

Uh oh!

richardlau commented Jul 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

puzpuzpuz commented Jul 30, 2020

Uh oh!

addaleax commented Jul 30, 2020

Uh oh!

addaleax commented Jul 30, 2020

Uh oh!

richardlau commented Jul 30, 2020

Uh oh!

addaleax commented Jul 31, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

richardlau commented Jul 30, 2020 •

edited

Loading