Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

src: fix unused namespace member in node_util #34565

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
Closed
puzpuzpuz wants to merge 1 commit into from
Closed

src: fix unused namespace member in node_util #34565

puzpuzpuz wants to merge 1 commit into from

Conversation

@puzpuzpuz
Copy link
Member

@puzpuzpuz puzpuzpuz commented Jul 30, 2020

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

puzpuzpuz commented Jul 30, 2020

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

@addaleax
Copy link
Member

addaleax commented Jul 30, 2020

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

addaleax commented Jul 30, 2020

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

@richardlau
Copy link
Member

richardlau commented Jul 30, 2020

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

👍 to both statements.

@addaleax
Copy link
Member

addaleax commented Jul 31, 2020

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 <himself65@outlook.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@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 <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
@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 <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
@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 <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
@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 <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
@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 <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 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 <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
@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 <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
@puzpuzpuz @addaleax
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
@puzpuzpuz @addaleax
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 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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.