FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[8.x] backport of util refactor and whatwg encoding by jasnell · Pull Request #14585 · nodejs/node · GitHub

/ node Public

[8.x] backport of util refactor and whatwg encoding - #14585

Closed
jasnell wants to merge 2 commits into
nodejs:v8.x-stagingfrom
jasnell:8x-backport-13644
ConversationCommits (2)ChecksFiles changed
Closed

[8.x] backport of util refactor and whatwg encoding#14585
jasnell wants to merge 2 commits into
nodejs:v8.x-stagingfrom
jasnell:8x-backport-13644

Conversation

jasnell commented Aug 1, 2017

Copy link
Copy Markdown
Member

Backport of #13803 and #13644 to v8.x-staging.

#13803 is required for #13644 to land cleanly.

This should go through a CI run before landing

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

util

jasnell added 2 commits August 1, 2017 16:24
util: refactor util module
7a3f0ef 
* refactor util exports
* early capture of prototype methods
* use template strings and args consistently

PR-URL: nodejs#13803
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
util: implement WHATWG Encoding Standard API
e1f4db6 
Provide an (initially experimental) implementation of the WHATWG Encoding
Standard API (`TextDecoder` and `TextEncoder`). The is the same API
implemented on the browser side.

By default, with small-icu, only the UTF-8, UTF-16le and UTF-16be decoders
are supported. With full-icu enabled, every encoding other than iso-8859-16
is supported.

This provides a basic test, but does not include the full web platform
tests. Note: many of the web platform tests for this would fail by default
because we ship with small-icu by default.

A process warning will be emitted on first use to indicate that the
API is still experimental. No runtime flag is required to use the
feature.

Refs: https://encoding.spec.whatwg.org/
PR-URL: nodejs#13644
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. v8.x labels Aug 1, 2017

jasnell commented Aug 1, 2017

Copy link
Copy Markdown
Member Author

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

jasnell requested a review from addaleax August 1, 2017 23:43

addaleax commented Aug 1, 2017

Copy link
Copy Markdown
Member

Thank you!

The second commit message doesn’t seem quite right, it should start with buffer: I think? LGTM apart from that.

addaleax commented Aug 1, 2017

Copy link
Copy Markdown
Member

Oh, that already happened during landing in master … hm. Should we fix that up here? I think our tooling would be okay with that and use buffer without making any more trouble.

jasnell commented Aug 1, 2017

Copy link
Copy Markdown
Member Author

Nope, the WHATWG Encoding stuff landed within util so the util: prefix is right. We need to revisit if that's the right place long term tho.

addaleax commented Aug 1, 2017

Copy link
Copy Markdown
Member

Oh, okay. In that case it’s just the original PR title that’s confusing. 😄

jasnell commented Aug 1, 2017

Copy link
Copy Markdown
Member Author

heh... oh right! completely forgot to change that!

addaleax pushed a commit that referenced this pull request Aug 2, 2017
util: refactor util module
f593960 
* refactor util exports
* early capture of prototype methods
* use template strings and args consistently

Backport-PR-URL: #14585
Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net>

PR-URL: #13803
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax pushed a commit that referenced this pull request Aug 2, 2017
util: implement WHATWG Encoding Standard API
9a3fc10 
Provide an (initially experimental) implementation of the WHATWG Encoding
Standard API (`TextDecoder` and `TextEncoder`). The is the same API
implemented on the browser side.

By default, with small-icu, only the UTF-8, UTF-16le and UTF-16be decoders
are supported. With full-icu enabled, every encoding other than iso-8859-16
is supported.

This provides a basic test, but does not include the full web platform
tests. Note: many of the web platform tests for this would fail by default
because we ship with small-icu by default.

A process warning will be emitted on first use to indicate that the
API is still experimental. No runtime flag is required to use the
feature.

Backport-PR-URL: #14585
Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net>

Refs: https://encoding.spec.whatwg.org/
PR-URL: #13644
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

addaleax commented Aug 2, 2017

Copy link
Copy Markdown
Member

CI failures are known flakes or infra problems

Landed in f593960 & 9a3fc10

addaleax closed this Aug 2, 2017
This file contains hidden or 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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

addaleax addaleax approved these changes

Assignees

No one assigned

Labels

lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL