doc: document and warn if the ICU version is too old #23766
Conversation
nodejs-github-bot
added
build
addaleax
added
the
author ready
configure.py
Outdated
There was a problem hiding this comment.
may? what can happen if it is? why not go all out and error?
(escape hatch is to manually edit config.gypi)
There was a problem hiding this comment.
probably a compile error. which they will see soon enough… ±0 on making it an error, either way.
configure.py
Outdated
There was a problem hiding this comment.
There was a problem hiding this comment.
@refack hm. Doesn't make sense with the format of current_ver.dep … the minimum ICU is global, doesn't need to be repeated multiple times.
There was a problem hiding this comment.
@refack I split the minimum version to a separate file.
There was a problem hiding this comment.
We could change the format of tools/icu/current_ver.dep to be:
{
"current_minimum": 57,
"packages": [ ... current content ... ]
}
|
CI (just in case you don't wanna patch this any more): https://ci.nodejs.org/job/node-test-pull-request/18060/ |
refack
removed
the
author ready
|
@refack how about moving the contents to the new .json file in a future PR once these land? (this and the 2 other PRs) |
Fixes: #19657 PR-URL: #23766 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]>
Fixes: #19657 PR-URL: #23766 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]>
|
This lands cleanly on 10.x, should we backport to 8.x? |
Fixes: #19657 PR-URL: #23766 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]>
Fixes: #19657 PR-URL: #23766 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Ujjwal Sharma <[email protected]>
8 participants
Fixes: #19657
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesExample: