src, doc: improve documentation and error message for ICU data fallback by joyeecheung · Pull Request #49666 · nodejs/node

joyeecheung · 2023-09-15T13:32:08Z

src: improve error message when ICU data cannot be initialized

Previously when we fail to initialize ICU data, the error message is

could not initialize ICU (check NODE_ICU_DATA or --icu-data-dir
parameters)

This patch updates it to something similar to:

U_FILE_ACCESS_ERROR: Could not initialize ICU. Check the directory
specified by NODE_ICU_DATA or --icu-data-dir contains icudt73l.dat
and it's readable

Where the expected data file name is the same as U_ICUDATA_NAME.

doc: improve documentation about ICU data fallback

This patch:

Documents --with-icu-default-data-dir and its precedence
Elaborates a bit more about the format of the name of the expected
data file.

nodejs-github-bot · 2023-09-15T13:32:13Z

Review requested:

@nodejs/startup

nodejs-github-bot · 2023-09-15T13:59:11Z

CI: https://ci.nodejs.org/job/node-test-pull-request/53964/

nodejs-github-bot · 2023-09-16T15:52:09Z

CI: https://ci.nodejs.org/job/node-test-pull-request/53986/

jasnell · 2023-09-16T15:54:18Z

Just curious, why std::string* and not const std::string&?

This is an out parameter, the caller gets the error string if it fails

nodejs-github-bot · 2023-09-17T22:08:11Z

CI: https://ci.nodejs.org/job/node-test-pull-request/54019/

joyeecheung · 2023-09-18T07:29:52Z

Fixed the line length to make the linter happy

nodejs-github-bot · 2023-09-18T07:31:48Z

CI: https://ci.nodejs.org/job/node-test-pull-request/54025/

Previously when we fail to initialize ICU data, the error message is ``` could not initialize ICU (check NODE_ICU_DATA or --icu-data-dir parameters) ``` This patch updates it to something similar to: ``` U_FILE_ACCESS_ERROR: Could not initialize ICU. Check the directory specified by NODE_ICU_DATA or --icu-data-dir contains icudt73l.dat and it's readable ``` Where the expected data file name is the same as U_ICUDATA_NAME.

This patch: - Documents `--with-icu-default-data-dir` and its precedence - Elaborates a bit more about the format of the name of the expected data file.

joyeecheung · 2023-09-18T10:33:33Z

Apparently test-icu-data-dir was matching the old error messages. Updated the test a bit.

joyeecheung · 2023-09-18T10:34:17Z

@richardlau @jasnell I've updated the test for new error message matches. Can you take a look again? Thanks

nodejs-github-bot · 2023-09-18T12:21:08Z

CI: https://ci.nodejs.org/job/node-test-pull-request/54034/

nodejs-github-bot · 2023-09-18T12:24:06Z

CI: https://ci.nodejs.org/job/node-test-pull-request/54035/

nodejs-github-bot · 2023-09-18T22:03:37Z

CI: https://ci.nodejs.org/job/node-test-pull-request/54044/

nodejs-github-bot · 2023-09-22T13:24:25Z

Landed in c2cd744...db5e993

Previously when we fail to initialize ICU data, the error message is ``` could not initialize ICU (check NODE_ICU_DATA or --icu-data-dir parameters) ``` This patch updates it to something similar to: ``` U_FILE_ACCESS_ERROR: Could not initialize ICU. Check the directory specified by NODE_ICU_DATA or --icu-data-dir contains icudt73l.dat and it's readable ``` Where the expected data file name is the same as U_ICUDATA_NAME. PR-URL: #49666 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com>

This patch: - Documents `--with-icu-default-data-dir` and its precedence - Elaborates a bit more about the format of the name of the expected data file. PR-URL: #49666 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Previously when we fail to initialize ICU data, the error message is ``` could not initialize ICU (check NODE_ICU_DATA or --icu-data-dir parameters) ``` This patch updates it to something similar to: ``` U_FILE_ACCESS_ERROR: Could not initialize ICU. Check the directory specified by NODE_ICU_DATA or --icu-data-dir contains icudt73l.dat and it's readable ``` Where the expected data file name is the same as U_ICUDATA_NAME. PR-URL: #49666 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com>

This patch: - Documents `--with-icu-default-data-dir` and its precedence - Elaborates a bit more about the format of the name of the expected data file. PR-URL: #49666 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Sep 15, 2023

richardlau approved these changes Sep 15, 2023
View reviewed changes

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 15, 2023

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 15, 2023

github-actionsBot mentioned this pull request Sep 16, 2023
CI Reliability 2023-09-16 nodejs/reliability#667
Open
25 tasks

jasnell reviewed Sep 16, 2023
View reviewed changes

jasnell approved these changes Sep 16, 2023
View reviewed changes

github-actionsBot mentioned this pull request Sep 17, 2023
CI Reliability 2023-09-17 nodejs/reliability#668
Open
32 tasks

github-actionsBot mentioned this pull request Sep 18, 2023
CI Reliability 2023-09-18 nodejs/reliability#669
Open
33 tasks

joyeecheung force-pushed the small-icu-doc branch from 81991de to ac4edacCompare September 18, 2023 07:29

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 18, 2023

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 18, 2023

joyeecheung added 2 commits September 18, 2023 12:32

doc: improve documentation about ICU data fallback
930aba9
This patch: - Documents `--with-icu-default-data-dir` and its precedence - Elaborates a bit more about the format of the name of the expected data file.

joyeecheung force-pushed the small-icu-doc branch from ac4edac to 930aba9Compare September 18, 2023 10:32

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 18, 2023

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 18, 2023

richardlau approved these changes Sep 18, 2023
View reviewed changes

This was referenced Sep 19, 2023
CI Reliability 2023-09-19 nodejs/reliability#670
Open
CI Reliability 2023-09-20 nodejs/reliability#671
Open
CI Reliability 2023-09-21 nodejs/reliability#672
Open

joyeecheung added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Sep 22, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 22, 2023

nodejs-github-bot closed this Sep 22, 2023

This was referenced Sep 28, 2023
v20.7.1 proposal #49917
Closed
v20.8.0 proposal #49932
Merged

Uh oh!

Uh oh!

Conversation

joyeecheung commented Sep 15, 2023

src: improve error message when ICU data cannot be initialized

doc: improve documentation about ICU data fallback

Uh oh!

nodejs-github-bot commented Sep 15, 2023

Uh oh!

nodejs-github-bot commented Sep 15, 2023

Uh oh!

nodejs-github-bot commented Sep 16, 2023

Uh oh!

jasnellSep 16, 2023

Choose a reason for hiding this comment

Uh oh!

joyeecheungSep 16, 2023

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Sep 17, 2023

Uh oh!

joyeecheung commented Sep 18, 2023

Uh oh!

nodejs-github-bot commented Sep 18, 2023

Uh oh!

joyeecheung commented Sep 18, 2023

Uh oh!

joyeecheung commented Sep 18, 2023

Uh oh!

nodejs-github-bot commented Sep 18, 2023

Uh oh!

nodejs-github-bot commented Sep 18, 2023

Uh oh!

nodejs-github-bot commented Sep 18, 2023

Uh oh!

nodejs-github-bot commented Sep 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants