doc: clarify use of deps/icu-small #43287
Closed
Conversation
This file contains 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
Add some additional info about use of icu subset in deps/icu-small Signed-off-by: Michael Dawson <mdawson@devrus.com>
|
Review requested:
|
|
This info would have helped me undestand more easily as I was confused by the overloading of small in terms of the data (small/full) and us using a subset of the C source code (deps/icu-small) @srl295 could you doulble check the change in the part about updating timezone data. I think that makese sense as I believe by default we use the data in deps/icu-small and that is what we would typically want to patch/commit as there is no deps/icu directory in the GitHub repo. |
srl295
reviewed
Jun 1, 2022
Co-authored-by: Steven R. Loomis <srl295@gmail.com>
|
@srl295 updated. |
|
I think the confusion is probably because:
|
srl295
approved these changes
Jun 1, 2022
srl295
reviewed
Jun 1, 2022
| @@ -11,6 +11,12 @@ functionality. To quote from icu-project.org: | |||
| > widely portable and gives applications the same results on all platforms | |||
| > and between C/C++ and Java software. | |||
|
|
|||
| If Node.js is configured to use its built-in ICU, | |||
| it uses a strict subset of ICU which is in | |||
There was a problem hiding this comment.
LGTM unless it's worth noting that the subset is not by locale (and type), but by type.
It's further confusing that 'small-icu' is an option still, but is !== the small-icu directory.
aduh95
reviewed
Jun 3, 2022
tniessen
reviewed
Jun 4, 2022
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Tobias Nießen <tniessen@tnie.de>
mhdawson
added a commit
that referenced
this pull request
Jun 14, 2022
Add some additional info about use of icu subset in deps/icu-small Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: #43287 Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
|
Landed in 83ffb79 |
danielleadams
pushed a commit
that referenced
this pull request
Jun 16, 2022
Add some additional info about use of icu subset in deps/icu-small Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: #43287 Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Merged
targos
pushed a commit
that referenced
this pull request
Jul 12, 2022
Add some additional info about use of icu subset in deps/icu-small Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: #43287 Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
targos
pushed a commit
that referenced
this pull request
Jul 31, 2022
Add some additional info about use of icu subset in deps/icu-small Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: #43287 Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
guangwong
pushed a commit
to noslate-project/node
that referenced
this pull request
Oct 10, 2022
Add some additional info about use of icu subset in deps/icu-small Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: nodejs/node#43287 Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add some additional info about use of icu
subset in deps/icu-small
Signed-off-by: Michael Dawson mdawson@devrus.com