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.

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

doc: list encodings supported by buffer.transcode #22263

Closed
wants to merge 2 commits into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Aug 11, 2018

Fixes: #15632

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@jasnell jasnell added buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. fast-track PRs that do not need to wait for 48 hours to land. labels Aug 11, 2018
@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. labels Aug 11, 2018
@jasnell jasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 11, 2018
@Trott Trott removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 11, 2018
@@ -2508,6 +2508,9 @@ encoding to another. Returns a new `Buffer` instance.
Throws if the `fromEnc` or `toEnc` specify invalid character encodings or if
conversion from `fromEnc` to `toEnc` is not permitted.

Encodings supported by `buffer.transcode()` include: `'ascii'`, `'utf8'`,
`'utf16le'`, `'ucs2'`, `'latin1'`, and `'binary'`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does 'hex' needs to be added here? (documentation)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. 'hex' is not supported

@@ -2508,6 +2508,9 @@ encoding to another. Returns a new `Buffer` instance.
Throws if the `fromEnc` or `toEnc` specify invalid character encodings or if
conversion from `fromEnc` to `toEnc` is not permitted.

Encodings supported by `buffer.transcode()` include: `'ascii'`, `'utf8'`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

include -> are assuming the list is supposed to be a complete list of supported encodings.

Copy link
Member Author

@jasnell jasnell Aug 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to make whatever editorial changes you'd like

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wanted to confirm the list is supposed to be complete before doing it. I'll take that as a "yes, it's a complete list" and make the change now. 👍

@Trott
Copy link
Member

Trott commented Aug 11, 2018

@nodejs/documentation Not sure if there's a desired formatting standard for "string values permitted for this argument must be from this small set". I don't think we have one, so I'm fine with this the way it is. Regardless, PTAL.

@Trott
Copy link
Member

Trott commented Aug 11, 2018

@nodejs/buffer

@vsemozhetbyt
Copy link
Contributor

Node.js Collaborators, please, add 👍 here if you approve fast-tracking.

jasnell added a commit that referenced this pull request Aug 12, 2018
Fixes: #15632

PR-URL: #22263
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@jasnell
Copy link
Member Author

jasnell commented Aug 12, 2018

Landed in a5b3c15 (fast tracked with approval)

@jasnell jasnell closed this Aug 12, 2018
targos pushed a commit that referenced this pull request Aug 12, 2018
Fixes: #15632

PR-URL: #22263
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. fast-track PRs that do not need to wait for 48 hours to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants