buffer: convert buffer.transcode to use internal/errors
#16352
Closed
Conversation
This file contains 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
nodejs-github-bot
added
the
buffer
targos
approved these changes
Oct 21, 2017
lib/buffer.js
Outdated
| @@ -1562,7 +1562,8 @@ if (process.binding('config').hasIntl) { | |||
| // Buffer instance. | |||
| transcode = function transcode(source, fromEncoding, toEncoding) { | |||
| if (!isUint8Array(source)) | |||
| throw new TypeError('"source" argument must be a Buffer or Uint8Array'); | |||
| throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'source', | |||
| ['buffer', 'uint8Array'], source); | |||
There was a problem hiding this comment.
please capitalize Buffer and Uint8Array.
There was a problem hiding this comment.
Most of the ERR_INVALID_ARG_TYPE errors in lib are using lower case.
For example:
https://github.com/nodejs/node/blob/master/lib/buffer.js#L198
https://github.com/nodejs/node/blob/master/lib/buffer.js#L225
https://github.com/nodejs/node/blob/master/lib/util.js#L965
Here is just for unitarity. It may need another PR to capitalize them.
There was a problem hiding this comment.
I think we should follow the convention of the docs here (lower case for primitives and capitals for others..)
There was a problem hiding this comment.
Those existing uses really ought to have been capitalized, we just missed them on the review, unfortunately.
lpinca
approved these changes
Oct 21, 2017
jasnell
approved these changes
Oct 22, 2017
`buffer.transcode` is still using raw TypeError. This change is to convert it to use internal/errors. Ref: nodejs#11273
starkwang
force-pushed
the
buffer-transcode-error
branch
from
da00b76
to
0117db7
Compare
|
Pushed commit to address comment. |
joyeecheung
approved these changes
Oct 22, 2017
gireeshpunathil
approved these changes
Oct 23, 2017
joyeecheung
added
the
errors
gireeshpunathil
pushed a commit
that referenced
this pull request
Oct 23, 2017
`buffer.transcode` is still using raw TypeError. This change is to convert it to use internal/errors. Ref: #11273 PR-URL: #16352 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>
|
Landed in e79a61c , thanks for the contribution! |
targos
added
the
semver-major
|
I added the |
addaleax
pushed a commit
to ayojs/ayo
that referenced
this pull request
Oct 26, 2017
`buffer.transcode` is still using raw TypeError. This change is to convert it to use internal/errors. Ref: nodejs/node#11273 PR-URL: nodejs/node#16352 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>
addaleax
pushed a commit
to ayojs/ayo
that referenced
this pull request
Dec 7, 2017
`buffer.transcode` is still using raw TypeError. This change is to convert it to use internal/errors. Ref: nodejs/node#11273 PR-URL: nodejs/node#16352 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
buffer.transcodeis still using raw TypeError. This change is to convert it to useinternal/errors.Ref: #11273
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
buffer