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

ICU-22922 Merge maint/maint-76 to main #3258

Merged
merged 6 commits into from
Nov 12, 2024
Merged

ICU-22922 Merge maint/maint-76 to main #3258

merged 6 commits into from
Nov 12, 2024

Conversation

sffc
Copy link
Member

@sffc sffc commented Oct 29, 2024

Checklist

  • Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22922
  • Required: The PR title must be prefixed with a JIRA Issue number. Example: "ICU-1234 Fix xyz"
  • Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
  • Required: Each commit message must be prefixed with a JIRA Issue number. Example: "ICU-1234 Fix xyz"
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

@sffc sffc requested a review from echeran October 30, 2024 00:13
@markusicu markusicu self-requested a review October 30, 2024 20:19
@eggrobin eggrobin removed the request for review from markusicu October 30, 2024 20:19
@markusicu
Copy link
Member

The list of commits looks fine. (I didn't check it against the maint branch.)
Can we do something about the exhautive-test failure? Does it just need a rerun?

@sffc
Copy link
Member Author

sffc commented Oct 30, 2024

Hmm, the exhaustive test failure is probably real

2024-10-21T21:56:04.7168279Z          TestDataDrivenLikelySubtags {
2024-10-21T21:56:04.7288800Z          addLikelySubtags('und-Latn-MU') should return 'en-Latn-MU' but got 'mfe-Latn-MU'
2024-10-21T21:56:04.7290073Z          minimizeSubtags('und-Latn-MU') should return 'en-MU' but got 'mfe'
2024-10-21T21:56:04.7291290Z          minimizeSubtags('und-Latn-MU') favor script should return 'en-MU' but got 'mfe'
2024-10-21T21:56:04.7293320Z          addLikelySubtags('und-Latn-RS') should return 'sr-Latn-RS' but got 'en-Latn-RS'
2024-10-21T21:56:04.7294596Z          minimizeSubtags('und-Latn-RS') should return 'sr-Latn' but got 'en-RS'
2024-10-21T21:56:04.7295896Z          minimizeSubtags('und-Latn-RS') favor script should return 'sr-Latn' but got 'en-RS'
2024-10-21T21:56:04.7297246Z          addLikelySubtags('und-Latn-SL') should return 'en-Latn-SL' but got 'kri-Latn-SL'
2024-10-21T21:56:04.7298463Z          minimizeSubtags('und-Latn-SL') should return 'en-SL' but got 'kri'
2024-10-21T21:56:04.7299960Z          minimizeSubtags('und-Latn-SL') favor script should return 'en-SL' but got 'kri'
2024-10-21T21:56:04.7301325Z          addLikelySubtags('und-Latn-TK') should return 'en-Latn-TK' but got 'tkl-Latn-TK'
2024-10-21T21:56:04.7302541Z          minimizeSubtags('und-Latn-TK') should return 'en-TK' but got 'tkl'
2024-10-21T21:56:04.7303759Z          minimizeSubtags('und-Latn-TK') favor script should return 'en-TK' but got 'tkl'
2024-10-21T21:56:04.7305069Z          addLikelySubtags('und-Latn-ZM') should return 'en-Latn-ZM' but got 'bem-Latn-ZM'
2024-10-21T21:56:04.7306275Z          minimizeSubtags('und-Latn-ZM') should return 'en-ZM' but got 'bem'
2024-10-21T21:56:04.7307462Z          minimizeSubtags('und-Latn-ZM') favor script should return 'en-ZM' but got 'bem'
2024-10-21T21:56:04.7332635Z       
2024-10-21T21:56:04.7334486Z          } ERRORS (15) in TestDataDrivenLikelySubtags (17ms) 

How should I proceed?

@sffc
Copy link
Member Author

sffc commented Oct 30, 2024

Actually it looks like the exhaustive failure is already failing on maint/maint-76

image

So I think we can ignore it for the purpose of merging.

@markusicu
Copy link
Member

These exhaustive-test failures looks related to the late CLDR change that got missed for ICU 76. We should have had a CLDR 46 beta4 and gotten that integrated...

Maybe let it fail and ask @DraganBesevic to do an integration of the CLDR 46 release into ICU main? Or even into maint/maint-76 and then merge that into main?

@macchiati @pedberg-icu FYI

@sffc
Copy link
Member Author

sffc commented Nov 4, 2024

As far as I can tell, the conclusion is currently:

Can we please get an integration of final CLDR 46 into ICU 76? Maybe into maint/maint-76 and then merge to main again? – agreed, Dragan will do today

which was posted on 2024-10-31. It looks like that PR is here: #3260

So I should wait for #3260 to merge and then see if I can merge this one?

@markusicu
Copy link
Member

As far as I can tell, the conclusion is currently:

Can we please get an integration of final CLDR 46 into ICU 76? Maybe into maint/maint-76 and then merge to main again? – agreed, Dragan will do today

which was posted on 2024-10-31. It looks like that PR is here: #3260

Yes, and I still have an open question why a CLDR data change is not making it into ICU.

So I should wait for #3260 to merge and then see if I can merge this one?

I think so, for now.
If we can't get the other PR resolved soon, then maybe merge what you have and prepare for later merging the full final CLDR update when we have it.

@markusicu
Copy link
Member

Hi @sffc the remaining issue is still open, but we should otherwise move on. Please merge this PR here -- with the special procedure for this kind of merge back to main.
@richgillam FYI

@sffc sffc merged commit 700c5e3 into main Nov 12, 2024
202 of 206 checks passed
@sffc
Copy link
Member Author

sffc commented Nov 12, 2024

I have merged the pull request with a merge commit.

I needed to change 2 settings in order to do this:

  1. Allowing merges by merge commit (this is the usual process)
  2. Allowing administrators to bypass branch protection, since the PR was not passing its status checks (this was a special thing I needed to do that hopefully I won't need to do next time)

I changed the settings back to the way they were after merging the PR.

@markusicu
Copy link
Member

Thank you!

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.

10 participants