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

v6.x backport: buffer,string_decoder: consolidate encoding validation logic #8463

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

@addaleax addaleax commented Sep 9, 2016

v6.x backport of #7207. Conflicts where with neighbouring, unrelated code both in lib/internal/util.js and test/parallel/test-buffer-alloc.js.

It’s only a cleanup change but other changes like a6f7b13 are beginning to depend on it.

CI: https://ci.nodejs.org/job/node-test-commit/4975/

/cc @jasnell

@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. string_decoder Issues and PRs related to the string_decoder subsystem. util Issues and PRs related to the built-in util module. v6.x labels Sep 9, 2016
@jasnell
Copy link
Member

jasnell commented Sep 9, 2016

LGTM

@Fishrock123
Copy link
Contributor

LGTM, thanks!

@Fishrock123 Fishrock123 force-pushed the v6.x-staging branch 2 times, most recently from b596da0 to 49f996f Compare September 9, 2016 15:19
Buffer.isEncoding and string_decoder.normalizeEncoding shared
quite a bit of logic. This moves the primary logic into
internal/util. The userland modules that monkey patch Buffer.isEncoding
should still work.

PR-URL: nodejs#7207
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@addaleax
Copy link
Member Author

addaleax commented Sep 9, 2016

@Fishrock123 … I guess you force-pushed to v6.x-staging or something? (edit: ah, yes, I see it in IRC now) I am rebasing this, but in case something goes wrong, 8141ff1 (the current HEAD here) should apply cleanly.

@Fishrock123
Copy link
Contributor

@addaleax see #node-dev... I had to twice. :S

@addaleax
Copy link
Member Author

addaleax commented Sep 9, 2016

yeah, no problem :)

Fishrock123 pushed a commit that referenced this pull request Sep 13, 2016
Buffer.isEncoding and string_decoder.normalizeEncoding shared
quite a bit of logic. This moves the primary logic into
internal/util. The userland modules that monkey patch Buffer.isEncoding
should still work.

PR-URL: #7207
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>

Refs: #8463
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@Fishrock123
Copy link
Contributor

Landed in dd51b1f, thanks!

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. string_decoder Issues and PRs related to the string_decoder subsystem. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants