doc: Update tools/icu/README.md

- 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>
nodejs · princjef · Oct 19, 2017 · Oct 27, 2017 · Oct 5, 2017 · Feb 19, 2018
commit 1af6ac2f1037d05918252252393e4625a81479f8
diff --git a/tools/icu/README.md b/tools/icu/README.md
@@ -1,4 +1,15 @@
-# Notes about the icu directory.
+# Notes about the `tools/icu` subdirectory
+
+This directory contains tools, data, and information about the [http://icu-project.org](ICU) (International Components for Unicode) integration. ICU is used to provide Intl functionality.
+
+- `patches/` are one-off patches, actually entire source file replacements, organized by ICU version number.
+- `icu_small.json` controls the "small" (English only) ICU. It is input to `icutrim.py`
+- `icu-generic.gyp` is the build file used for most ICU builds within ICU. <!-- have fun -->
+- `icu-system.gyp` is an alternate build file used when `--with-intl=system-icu` is invoked. It builds against the `pkg-config` located ICU.
+- `iculslocs.cc` is source for the `iculslocs` utility, invoked by `icutrim.py` as part of repackaging. Not used separately. See source for more details.
+- `no-op.cc` — empty function to convince gyp to use a C++ compiler.
+- `README.md` — you are here
+- `shrink-icu-src.py` — this is used during upgrade (see guide below)
 
 ## How to upgrade ICU
 
@@ -12,13 +23,13 @@
 make
 ```
 
-(The equivalent `vcbuild.bat` commands should work also. Note that we use the `.tgz` and not the `.zip` here,
-that is because of line endings.)
+> _Note_ in theory, the equivalent `vcbuild.bat` commands should work also,
+but the commands below are makefile-centric.
 
-- (note- may need to make changes in `icu-generic.gyp` or `tools/icu/patches` for
-version specific stuff)
+- If there are ICU version-specific changes needed, you may need to make changes in `icu-generic.gyp` or add patch files to `tools/icu/patches`.
+  - Specifically, look for the lists in `sources!` in the `icu-generic.gyp` for files to exclude.
 
-- Verify the node build works
+- Verify the node build works:
 
 ```shell
 make test-ci
@@ -27,13 +38,12 @@ make test-ci
 Also running
 
 <!-- eslint-disable strict -->
+
 ```js
 new Intl.DateTimeFormat('es', {month: 'long'}).format(new Date(9E8));
 ```
 
 …Should return `January` not `enero`.
-(TODO here: improve [testing](https://github.com/nodejs/Intl/issues/16))
-
 
 - Now, copy `deps/icu` over to `deps/icu-small`
 
@@ -43,25 +53,25 @@ python tools/icu/shrink-icu-src.py
 
 - Now, do a clean rebuild of node to test:
 
-(TODO: fix this when these options become default)
-
 ```shell
-./configure --with-intl=small-icu --with-icu-source=deps/icu-small
+make -k distclean
+./configure
 make
 ```
 
 - Test this newly default-generated Node.js
 
 <!-- eslint-disable strict -->
+
 ```js
 process.versions.icu;
 new Intl.DateTimeFormat('es', {month: 'long'}).format(new Date(9E8));
 ```
 
-(should return your updated ICU version number, and also `January` again.)
+(This should print your updated ICU version number, and also `January` again.)
 
-- You are ready to check in the updated `deps/small-icu`.
-This is a big commit, so make this a separate commit from other changes.
+You are ready to check in the updated `deps/small-icu`. This is a big commit,
+so make this a separate commit from the smaller changes.
 
 - Now, rebuild the Node license.
 
@@ -85,30 +95,27 @@ make test-ci
 
 - commit the change to `configure` along with the updated `LICENSE` file.
 
+  - Note: To simplify review, I often will “pre-land” this patch, meaning that I run the patch through `curl -L https://github.com/nodejs/node/pull/xxx.patch | git am -3 --whitespace=fix` per the collaborator’s guide… and then push that patched branch into my PR's branch. This reduces the whitespace changes that show up in the PR, since the final land will eliminate those anyway.
+
 -----
 
-## Notes about these tools
+## Postscript about the tools
 
-The files in this directory were written for the node.js effort. It's
-the intent of their author (Steven R. Loomis / srl295) to merge them
-upstream into ICU, pending much discussion within the ICU-PMC.
+The files in this directory were written for the node.js effort.
+It was the intent of their author (Steven R. Loomis / srl295) to
+merge them upstream into ICU, pending much discussion within the
+ICU-TC.
 
 `icu_small.json` is somewhat node-specific as it specifies a "small ICU"
 configuration file for the `icutrim.py` script. `icutrim.py` and
 `iculslocs.cpp` may themselves be superseded by components built into
-ICU in the future.
-
-The following tickets were opened during this work, and their
-resolution may inform the reader as to the current state of icu-trim
-upstream:
+ICU in the future. As of this writing, however, the tools are separate
+entities within Node, although theyare being scrutinized by interested
+members of the ICU-TC. The “upstream” ICU bugs are given below.
 
    * [#10919](http://bugs.icu-project.org/trac/ticket/10919)
      (experimental branch - may copy any source patches here)
    * [#10922](http://bugs.icu-project.org/trac/ticket/10922)
      (data packaging improvements)
    * [#10923](http://bugs.icu-project.org/trac/ticket/10923)
      (rewrite data building in python)
-
-When/if components (not including the `.json` file) are merged into
-ICU, this code and `configure` will be updated to detect and use those
-variants rather than the ones in this directory.