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

Fix incorrect use of ERR_INVALID_ARG_TYPE #14011

Closed
wants to merge 4 commits into from
Closed

Fix incorrect use of ERR_INVALID_ARG_TYPE #14011

wants to merge 4 commits into from

Conversation

tniessen
Copy link
Member

These lines effectively printed typeof typeof value, which is always string, instead of typeof value.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

path, assert

@tniessen tniessen added assert Issues and PRs related to the assert subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. path Issues and PRs related to the path subsystem. labels Jun 30, 2017
@tniessen tniessen self-assigned this Jun 30, 2017
@nodejs-github-bot nodejs-github-bot added assert Issues and PRs related to the assert subsystem. path Issues and PRs related to the path subsystem. labels Jun 30, 2017
@tniessen
Copy link
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/8902/

@cjihrig
Copy link
Contributor

cjihrig commented Jun 30, 2017

Are there any tests that can be updated to prevent regressions on these?

@tniessen
Copy link
Member Author

tniessen commented Jun 30, 2017 •
edited
Loading

@cjihrig I don't think there is test coverage for that, but I can add those tests.

@tniessen
Copy link
Member Author

CI with tests: https://ci.nodejs.org/job/node-test-pull-request/8904/

@BridgeAR
Copy link
Member

Ref #13834

@cjihrig cjihrig mentioned this pull request Jul 1, 2017
Closed
3 tasks
@tniessen
Copy link
Member Author

tniessen commented Jul 3, 2017

Pre-land CI: https://ci.nodejs.org/job/node-test-commit/10920/

tniessen added a commit that referenced this pull request Jul 3, 2017
@tniessen
assert: fix incorrect use of ERR_INVALID_ARG_TYPE
1d74143 
PR-URL: #14011
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
tniessen added a commit that referenced this pull request Jul 3, 2017
@tniessen
path: fix incorrect use of ERR_INVALID_ARG_TYPE
44256bb 
PR-URL: #14011
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@tniessen
Copy link
Member Author

tniessen commented Jul 3, 2017 •
edited
Loading

Landed in 1d74143 44256bb.

@tniessen tniessen closed this Jul 3, 2017
@addaleax
Copy link
Member

addaleax commented Jul 3, 2017

This doesn’t land cleanly on 8.x; if you can, please follow the guide and raise a backport PR.

@tniessen
Copy link
Member Author

tniessen commented Jul 3, 2017

@addaleax I think dcfbbac should be backported first. These changes won't land cleanly without it.

@addaleax
Copy link
Member

@tniessen That’s a semver-major change, we can’t backport that :(

@tniessen
Copy link
Member Author

@addaleax Right, then it does not make sense to backport this PR ;)

@addaleax
Copy link
Member

Fwiw I didn’t notice these were independent commits here, the assert one should land on 8.x.

addaleax pushed a commit to addaleax/node that referenced this pull request Jul 24, 2017
@tniessen @addaleax
assert: fix incorrect use of ERR_INVALID_ARG_TYPE
11115fe 
PR-URL: nodejs#14011
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@addaleax addaleax mentioned this pull request Jul 24, 2017
Closed
tniessen added a commit to tniessen/node that referenced this pull request Jul 25, 2017
@tniessen
assert: fix incorrect use of ERR_INVALID_ARG_TYPE
24374d8 
PR-URL: nodejs#14011
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@tniessen tniessen mentioned this pull request Jul 25, 2017
Closed
addaleax pushed a commit to addaleax/node that referenced this pull request Jul 27, 2017
@tniessen @addaleax
assert: fix incorrect use of ERR_INVALID_ARG_TYPE
89586f6 
Backport-PR-URL: nodejs#14459
Backport-Reviewed-By: Refael Ackermann <refack@gmail.com>

PR-URL: nodejs#14011
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@addaleax addaleax mentioned this pull request Aug 2, 2017
Merged
@jasnell jasnell mentioned this pull request Oct 30, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@refack refack refack approved these changes

@jasnell jasnell jasnell approved these changes

@lpinca lpinca lpinca approved these changes

@cjihrig cjihrig cjihrig approved these changes

Assignees

@tniessen tniessen

Labels
assert Issues and PRs related to the assert subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. path Issues and PRs related to the path subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@tniessen @cjihrig @BridgeAR @addaleax @refack @jasnell @lpinca @nodejs-github-bot