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

doc: Update tools/icu/README.md by srl295 · Pull Request #16939 · nodejs/node · GitHub

/ node Public

doc: Update tools/icu/README.md - #16939

Closed
srl295 wants to merge 0 commit into
nodejs:masterfrom
srl295:update-icu-guide
ConversationCommits (0)ChecksFiles changed
Closed

doc: Update tools/icu/README.md#16939
srl295 wants to merge 0 commit into
nodejs:masterfrom
srl295:update-icu-guide

Conversation

srl295 commented Nov 10, 2017 •
edited
Loading

Copy link
Copy Markdown
Member
  • 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: #7843

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)

srl295 added the i18n-api Issues and PRs related to the i18n implementation. label Nov 10, 2017
srl295 self-assigned this Nov 10, 2017
nodejs-github-bot added i18n-api Issues and PRs related to the i18n implementation. tools Issues and PRs related to the tools directory. labels Nov 10, 2017
mathiasbynens reviewed Nov 12, 2017
View reviewed changes
Comment thread tools/icu/README.md Outdated

mathiasbynens Nov 12, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nit: not just Intl.* — it‘s also used for e.g. RegExp Unicode property escapes.

addaleax Nov 18, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@mathiasbynens Would just using internationalization functionality instead of Intl functionality sound good to you?

mathiasbynens Nov 19, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yeah, that sounds good!

srl295 Jan 30, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

(dramatic pause) 👍

mhdawson approved these changes Nov 16, 2017
View reviewed changes

mhdawson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

apapirovski commented Dec 9, 2017

Copy link
Copy Markdown
Contributor

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.

BridgeAR commented Jan 19, 2018

Copy link
Copy Markdown
Member

New Mini-CI (seemed like the old one failed?): https://ci.nodejs.org/job/node-test-commit-light/148/

BridgeAR added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jan 19, 2018
srl295 added a commit that referenced this pull request Jan 30, 2018
doc: Update tools/icu/README.md
07712d1 
- 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: #7843

PR-URL: #16939
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
srl295 closed this Jan 30, 2018
srl295 deleted the update-icu-guide branch January 30, 2018 00:35
apapirovski mentioned this pull request Jan 30, 2018
Closed
2 tasks
MylesBorins pushed a commit that referenced this pull request Feb 20, 2018
doc: Update tools/icu/README.md
f86f6a7 
- 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: #7843

PR-URL: #16939
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
doc: Update tools/icu/README.md
8ce2368 
- 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: #7843

PR-URL: #16939
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
doc: Update tools/icu/README.md
2d968ca 
- 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: #7843

PR-URL: #16939
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins mentioned this pull request Feb 21, 2018
Merged

MylesBorins commented Mar 20, 2018

Copy link
Copy Markdown
Contributor

Does this need to be backported?

MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
doc: Update tools/icu/README.md
92c6b9c 
- 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>
MylesBorins pushed a commit that referenced this pull request May 23, 2018
doc: Update tools/icu/README.md
1af6ac2 
- 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: #7843

PR-URL: #16939
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Jun 14, 2018
doc: Update tools/icu/README.md
21de715 
- 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: #7843

PR-URL: #16939
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins mentioned this pull request Jul 9, 2018
Merged
rvagg pushed a commit that referenced this pull request Aug 16, 2018
doc: Update tools/icu/README.md
fceeee6 
- 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: #7843

PR-URL: #16939
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This was referenced Mar 9, 2020
Open
Open
snyk-bot mentioned this pull request Jul 15, 2022
Open
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 left review comments
jasnell jasnell approved these changes
+2 more reviewers
mathiasbynens mathiasbynens left review comments
mhdawson mhdawson approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

srl295

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. i18n-api Issues and PRs related to the i18n implementation. tools Issues and PRs related to the tools directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

deps: fix TODO in tools/icu/README.md

9 participants

Back | FazBrowse Home | New Git URL