Skip to content
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

CLDR-17484 Update dayPeriods.xml #4375

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

macchiati
Copy link
Member

One small change from the ticket to minimize changes: keeping the boundary between evening and night at 9:00pm

CLDR-17484

  • This PR completes the ticket.

ALLOW_MANY_COMMITS=true

One small change from the ticket to minimize changes: keeping the boundary between evening and night at 9:00pm
richgillam
richgillam previously approved these changes Feb 13, 2025
Copy link
Contributor

@richgillam richgillam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

richgillam
richgillam previously approved these changes Feb 14, 2025
@macchiati
Copy link
Member Author

Still fixing tests; don't review until they complete successfully. Should have marked as draft until the tests complete.

@macchiati macchiati requested a review from richgillam February 20, 2025 00:23
@macchiati
Copy link
Member Author

Finally fixed the test failure, so we can merge this. (We already agreed that this was very unlikely to cause ICU problem [Although there might be unit tests that need adjustment, like I had to for CLDR]).

Copy link
Contributor

@richgillam richgillam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with the CLDR tests, but the changes there look plausible, and the CLDR data changes look good.

@macchiati macchiati marked this pull request as ready for review February 20, 2025 04:29
@macchiati
Copy link
Member Author

@markusicu, while you were out I checked with Rich and Dragan about this one, and said it was ok. But before merging I want to check with you.

@markusicu
Copy link
Member

thanks for checking; these changes are ok with me

@pedberg-icu
Copy link
Contributor

pedberg-icu commented Feb 20, 2025

Also discussed in ICU TC, and agreed this is OK for CLDR 77. However ICU will likely create release candidate before this is integrated (so this would go in after that, before final release)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants