-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
doc: Update tools/icu/README.md #16939
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
Conversation
tools/icu/README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: not just Intl.* — it‘s also used for e.g. RegExp Unicode property escapes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mathiasbynens Would just using internationalization functionality instead of Intl functionality sound good to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that sounds good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(dramatic pause) 👍
mhdawson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
CI: https://ci.nodejs.org/job/node-test-pull-request-lite/10/ I would like to land this soon so will implement the change suggested by @mathiasbynens myself. It's been sitting around for almost a month now due to that tiny little thing. |
|
New Mini-CI (seemed like the old one failed?): https://ci.nodejs.org/job/node-test-commit-light/148/ |
56d88de to
2343bd8
Compare
2343bd8 to
bd36a36
Compare
bd36a36 to
34cbce9
Compare
34cbce9 to
07712d1
Compare
|
Does this need to be backported? |
- remove TODOs: the one about defaults has been addressed, and the one about testing is a work item that doesn't belong in a doc. - add some background information Fixes: nodejs#7843 PR-URL: nodejs#16939 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
addressed, and the one about testing is a work
item that doesn't belong in a doc.
Fixes: #7843
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)