-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8346944: Update Unicode Data Files to 17.0.0 #28093
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.
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
Conversation
|
/issue add JDK-8346947 |
|
👋 Welcome back naoto! A progress list of the required criteria for merging this PR into |
|
@naotoj This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 6 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
|
@naotoj |
e1a9e29 to
d9cff00
Compare
Webrevs
|
JoeWang-Java
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
| */ | ||
| @Deprecated | ||
| public static final String ICU_DATA_VERSION_PATH = "76b"; | ||
| public static final String ICU_DATA_VERSION_PATH = "78b"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now with fixed path "icudata", this seems to be no longer used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. The change was in the upstream which was just merged to our codebase: unicode-org/icu@d93b8bb#diff-2d49053b635edae3d63d88a8b058994e7b156a50d5056ac0fb3bbfe5daa7c4e4L242
|
Thanks for the review! |
|
Going to push as commit 4a97563.
Your commit was automatically rebased without conflicts. |
justin-curtis-lu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. (Looks like I missed integration by a few minutes; perhaps my comment can go into Unicode 18)
| */ | ||
| public static final class UnicodeBlock extends Subset { | ||
| /** | ||
| * NUM_ENTITIES should match the total number of UnicodeBlocks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought at first NUM_ENTITIES should be 790 due to 8 new UnicodeBlocks being added. However, it does go up to 804, becasue the total number includes the identifiers and aliases (which is still a single UnicodeBlock). I think that distinction is worth correcting in the comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Filed: https://bugs.openjdk.org/browse/JDK-8372117
Updates the JDK to use the latest Unicode 17.0.0, which also updates the ICU4J along with it (8346947
Update ICU4J to Version 78.1). The corresponding CSR has already been approved.
Progress
Warnings
Issues
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28093/head:pull/28093$ git checkout pull/28093Update a local copy of the PR:
$ git checkout pull/28093$ git pull https://git.openjdk.org/jdk.git pull/28093/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 28093View PR using the GUI difftool:
$ git pr show -t 28093Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28093.diff
Using Webrev
Link to Webrev Comment