-
-
Notifications
You must be signed in to change notification settings - Fork 847
ICU-23278 Fix ldml2icu metazones #3784
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
0bce576 to
b57c970
Compare
|
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
b57c970 to
cbc114b
Compare
|
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
|
Thanks for this pull request! |
|
This fully fixes the problem. The display name for the GMT meta zone already needs to be in the data for other zones. Where should a unit test go? |
cbc114b to
2f7430c
Compare
|
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
|
Ok I've written two unit tests, however these don't seem to fail without the fix. I suspect it might use the file at |
|
@roubert or @FrankYFTang could you please see whether the fuzzer failure is real and related to these changes? seems unlikely but blocks this PR :-( |
|
I do not believe the fuzzer breakage is realted to this PR. I filed another bug https://unicode-org.atlassian.net/browse/ICU-23283 to look into that. |
|
I build the fuzzer in trunk w/ the patch and run the fuzzer and cnnot reproduce the issue. Try to run it on branch in action w/o the PR now |
|
ok, I think what happen is that issue is fixed by me on 59577f6 but that is not in the branch |
|
somehow I cannot reproduce the fuzzer issue on https://github.com/unicode-org/icu/actions/runs/19944874424 |
|
Hi @robertbastian -- Frank's fix is now cherry-picked onto the maintenance branch. Please rebase this PR, which should get CI to pass, and then you should be able to merge. |
2f7430c to
4b40fa8
Compare
|
Hooray! The files in the branch are the same across the force-push. 😃 ~ Your Friendly Jira-GitHub PR Checker Bot |
markusicu
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.
CI checks pass!
@robertbastian please merge. (You didn't check the Feel free to merge on my behalf box :-)
Updated
ldml2icu_supplemental.txtand ran steps 5b and 12 of the integration checklist.Checklist