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.

Sign up for GitHub

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

Jump to bottom

src: fix unused namespace member in node_util #34565

Closed
wants to merge 1 commit into from
Closed

src: fix unused namespace member in node_util #34565

wants to merge 1 commit into from

Conversation

puzpuzpuz
Copy link
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 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
Copy link
Member

richardlau commented Jul 30, 2020 •
edited
Loading

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

@puzpuzpuz
Copy link
Member Author

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

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

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

@richardlau
Copy link
Member

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

👍 to both statements.

@addaleax
Copy link
Member

Landed in 1e47051

@addaleax addaleax closed this Jul 31, 2020
addaleax pushed a commit that referenced this pull request Jul 31, 2020
@puzpuzpuz @addaleax
src: fix unused namespace member in node_util
1e47051 
PR-URL: #34565
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@puzpuzpuz 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
@addaleax
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 addaleax mentioned this pull request Jul 31, 2020
Closed
3 tasks
codebytere pushed a commit that referenced this pull request Aug 5, 2020
@puzpuzpuz @codebytere
src: fix unused namespace member in node_util
c1abc8d 
PR-URL: #34565
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
jasnell pushed a commit that referenced this pull request Aug 7, 2020
@addaleax @jasnell
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
jasnell pushed a commit that referenced this pull request Aug 7, 2020
@addaleax @jasnell
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
addaleax added a commit that referenced this pull request Aug 8, 2020
@addaleax
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
addaleax added a commit that referenced this pull request Aug 8, 2020
@addaleax
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@codebytere codebytere mentioned this pull request Aug 10, 2020
Merged
codebytere pushed a commit that referenced this pull request Aug 11, 2020
@addaleax @codebytere
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
codebytere pushed a commit that referenced this pull request Aug 11, 2020
@addaleax @codebytere
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 <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
@puzpuzpuz @addaleax
src: fix unused namespace member in node_util
761c6f4 
PR-URL: #34565
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
@puzpuzpuz @addaleax
src: fix unused namespace member in node_util
c0a961e 
PR-URL: #34565
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@codebytere codebytere mentioned this pull request Sep 28, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@richardlau richardlau richardlau approved these changes

@himself65 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
@puzpuzpuz @richardlau @addaleax @jasnell @himself65 @nodejs-github-bot