Skip to content

Adapt ext/intl tests for ICU 72.1 #9800

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

Closed
wants to merge 2 commits into from
Closed

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Oct 21, 2022

This version replaces SPACEs before the meridian with NARROW NO-BREAK SPACEs. Thus, we split the affected test cases as usual.

Fixes GH-9799.

This version replaces SPACEs before the meridian with NARROW NO-BREAK
SPACEs.  Thus, we split the affected test cases as usual.
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Maybe instead of adding numbers to the file name having something like _icu72-1 at the end is better so that at a glance one can see why there are different variants?

@cmb69
Copy link
Member Author

cmb69 commented Oct 21, 2022

Maybe instead of adding numbers to the file name having something like _icu72-1 at the end is better so that at a glance one can see why there are different variants?

That makes sense, but how should we suffix files which are for ICU x - ICU y? There are quite a lot of these. Or apply that naming scheme only to new files?

@cmb69 cmb69 marked this pull request as ready for review October 21, 2022 16:50
@Girgias
Copy link
Member

Girgias commented Oct 21, 2022

Maybe instead of adding numbers to the file name having something like _icu72-1 at the end is better so that at a glance one can see why there are different variants?

That makes sense, but how should we suffix files which are for ICU x - ICU y? There are quite a lot of these. Or apply that naming scheme only to new files?

Hum, maybe if there is a version number X than the test is for that version and above. However if there is another copy of the test with a higher version number Y than the test for version above X becomes between version X and Y?
Therefore we wouldn't need to rename test files if another incompatibility arises.

@andypost
Copy link
Contributor

Thank you! that fixes build, at least all upgraded arches are pass now https://gitlab.alpinelinux.org/alpine/aports/-/pipelines/140943

@cmb69 cmb69 closed this in 8dd51b4 Oct 22, 2022
@cmb69 cmb69 deleted the cmb/icu-72.1 branch October 22, 2022 08:33
cmb69 added a commit to cmb69/php-src that referenced this pull request Oct 31, 2024
Regarding the test names, see PR php#9800.
cmb69 added a commit that referenced this pull request Nov 1, 2024
Regarding the test names, see PR #9800.

Closes GH-16660.
cmb69 added a commit to cmb69/php-src that referenced this pull request Nov 13, 2024
Regarding the test names, see PR php#9800.

Closes phpGH-16660.

(cherry picked from commit 3245a65)
cmb69 added a commit to cmb69/php-src that referenced this pull request Nov 13, 2024
Regarding the test names, see PR php#9800.

(cherry picked from commit 3245a65)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

compatibility with ICU 72.1
3 participants