FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

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

/ node Public

src: fix unused namespace member in node_util - #34565

Closed
puzpuzpuz wants to merge 1 commit into
nodejs:masterfrom
puzpuzpuz:cleanup/remove-unused-import-from-node-util
ConversationCommits (1)ChecksFiles changed
Closed

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

Conversation

puzpuzpuz commented Jul 30, 2020

Copy link
Copy Markdown
Member

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

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

richardlau commented Jul 30, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

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

puzpuzpuz commented Jul 30, 2020

Copy link
Copy Markdown
Member Author

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

addaleax commented Jul 30, 2020

Copy link
Copy Markdown
Member

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 added the fast-track PRs that do not need to wait for 48 hours to land. label Jul 30, 2020

addaleax commented Jul 30, 2020

Copy link
Copy Markdown
Member

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

richardlau commented Jul 30, 2020

Copy link
Copy Markdown
Member

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

👍 to both statements.

addaleax commented Jul 31, 2020

Copy link
Copy Markdown
Member

Landed in 1e47051

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 added a commit to addaleax/node that referenced this pull request Jul 31, 2020
tools: fix C++ import checker argument expansion
22c467d 
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
addaleax mentioned this pull request Jul 31, 2020
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>
jasnell pushed a commit that referenced this pull request Aug 7, 2020
src: fix linter failures
bfebfdb 
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>
jasnell pushed a commit that referenced this pull request Aug 7, 2020
tools: fix C++ import checker argument expansion
318c982 
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>
addaleax added a commit that referenced this pull request Aug 8, 2020
src: fix linter failures
2dbd15a 
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>
addaleax added a commit that referenced this pull request Aug 8, 2020
tools: fix C++ import checker argument expansion
05100e1 
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>
codebytere mentioned this pull request Aug 10, 2020
Merged
codebytere pushed a commit that referenced this pull request Aug 11, 2020
src: fix linter failures
e97fe4b 
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>
codebytere pushed a commit that referenced this pull request Aug 11, 2020
tools: fix C++ import checker argument expansion
af0cfeb 
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>
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
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
richardlau richardlau approved these changes
himself65 himself65 approved these changes

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. fast-track PRs that do not need to wait for 48 hours to land. util Issues and PRs related to the built-in util module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL