Skip to content

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

@puzpuzpuzpuzpuzpuz 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-botnodejs-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
MemberAuthor

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

@addaleaxaddaleax 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 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]>
@puzpuzpuzpuzpuzpuz 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
@addaleaxaddaleax 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]>
@codebyterecodebytere 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]>
@codebyterecodebytere mentioned this pull request Sep 28, 2020
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes

@richardlaurichardlaurichardlau approved these changes

@himself65himself65himself65 approved these changes

Assignees

No one assigned

Labels

c++Issues and PRs that require attention from people who are familiar with C++.fast-trackPRs that do not need to wait for 48 hours to land.utilIssues 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