Skip to content

Adapt ext/intl tests for ICU 76.1 #16788

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 3 commits into from
Closed

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Nov 13, 2024

Regarding the test names, see PR #9800.

(cherry picked from commit 3245a65)


macOS CI is now using ICU 76.1 so we need to backport the test fix. I should probably have targeted PHP-8.2 in the first place.

Regarding the test names, see PR php#9800.

(cherry picked from commit 3245a65)
@cmb69 cmb69 requested a review from devnexen as a code owner November 13, 2024 21:14
@cmb69 cmb69 marked this pull request as draft November 13, 2024 21:39
(cherry picked from commit e111bf7)
Apparently, the error message may contain the called function.
@cmb69 cmb69 marked this pull request as ready for review November 14, 2024 01:15
@cmb69
Copy link
Member Author

cmb69 commented Nov 14, 2024

I also backported the test fixes for ICU 75.1, and had to adapt a new test case.

@iluuu1994
Copy link
Member

Shouldn't this target 8.1 instead?

@cmb69
Copy link
Member Author

cmb69 commented Nov 14, 2024

I think we need to lock 8.1 to ICU 74 anyway, since otherwise we would need to introduce support for C++17 into the security branch (see #16789). I doubt that we should do that (user can update to PHP 8.2 or later, if they want to use latest ICU versions). And if we don't support ICU >= 75 for PHP-8.1, there is no need to update the tests.

@cmb69 cmb69 closed this in f725f50 Nov 15, 2024
@cmb69 cmb69 deleted the cmb/icu-76.1 branch November 15, 2024 17:58
ericmann pushed a commit that referenced this pull request Nov 19, 2024
adoy pushed a commit that referenced this pull request Nov 19, 2024
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.

2 participants