-
-
Notifications
You must be signed in to change notification settings - Fork 767
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
ICU-23033 Regenerate scx value array #3355
Conversation
I haven't tested the output, please do check that the codepoints are the right ones. |
8539c7a
to
684bede
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
684bede
to
05377a0
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
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 thanks
This PR does not fix ICU-21821 and therefore should not be tagged with that issue number. Is there a rush for this? I would rather see the actual fix. There's no good way to review this PR and we rely on the astute observations of people like @FrankYFTang to catch issues. |
@sffc we have clients reporting discrepancies in their data because of this. The correct fix involves adding APIs to ICU4C, unless that can be done in a short period of time, I think we should land this and get a new icuexportdata. Or we should somehow patch this data into 2.0.0-beta1 datagen: the goal is to get updated data released, it's not important if icuexportdata is fixed in the meantime. I'm happy to rewrite the Python code to be a bit more readable and reviewable. |
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.
Agree w/ @sffc, please file a new ticket and change the PR to use that new ticket
05377a0
to
0b3e9be
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
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
@sffc can you do the uplift dance to get a tagged release? I forgot what needs to be done, is it just a cherry-pick PR to |
I think we should just tag from the main branch. It has CLDR 47, but we should be integrating CLDR 47 into ICU4X within the next week anyway. CC @echeran who usually does this. |
This fixes the scx array hardcoded in icuexportdata for Unicode 16 (see: unicode-org/icu4x#6041)
This is a quick and dirty fix, I've tried to include the python script I used to generate this. We should still fix ICU-21821.
Checklist