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.

Sign up for GitHub

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

Jump to bottom

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

Closed
srl295 opened this issue Jul 22, 2016 · 3 comments
Closed

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

srl295 opened this issue Jul 22, 2016 · 3 comments
Assignees
@srl295
Labels
doc Issues and PRs related to the documentations. i18n-api Issues and PRs related to the i18n implementation.

Comments

@srl295
Copy link
Member

srl295 commented Jul 22, 2016

  • Subsystem: deps

Fix TODO in https://github.com/nodejs/node/blob/master/tools/icu/README.md

small-icu is the default now.
@srl295 srl295 self-assigned this Jul 22, 2016
@srl295 srl295 added i18n-api Issues and PRs related to the i18n implementation. doc Issues and PRs related to the documentations. labels Jul 22, 2016
@srl295
Copy link
Member Author

srl295 commented Jul 22, 2016 •
edited
Loading

  • watch out for deps/icu-small/source/data/in/icudt*.dat :
  1. that's where a 2MB binary file will be - make sure to git add it
  2. don't forget to delete the old ICU's file

@Trott
Copy link
Member

Trott commented Jul 9, 2017

/ping @srl295 There are two TODOs in the icu README. Are either of them addressable at this time?

@srl295
Copy link
Member Author

srl295 commented Nov 10, 2017

also:

it should mention pre-landing to simplify the diffs (with fix-whitespace - what I do is actually "pre-land" the patch using the Collaborator's guide onto a fresh master, then push that again otherwise the PR diff shows gratuitous whitespace diffs that aren't really part of the final land)
it should also cleanup the order of operations and mention running the license generator after 'make distclean ; configure ' so that the path to ICU is consistent

This was referenced Nov 10, 2017
Closed
Closed
@srl295 srl295 closed this as completed in 07712d1 Jan 30, 2018
MylesBorins pushed a commit that referenced this issue Feb 20, 2018
@srl295 @MylesBorins
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 <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue Feb 21, 2018
@srl295 @MylesBorins
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 <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue Feb 21, 2018
@srl295 @MylesBorins
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 <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MayaLekova pushed a commit to MayaLekova/node that referenced this issue May 8, 2018
@srl295 @MayaLekova
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 <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue May 23, 2018
@srl295 @MylesBorins
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 <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue Jun 14, 2018
@srl295 @MylesBorins
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 <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
rvagg pushed a commit that referenced this issue Aug 16, 2018
@srl295 @rvagg
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 <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@srl295 srl295

Labels
doc Issues and PRs related to the documentations. i18n-api Issues and PRs related to the i18n implementation.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

doc: Update tools/icu/README.md srl295/node
2 participants
@Trott @srl295