Skip to content

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Oct 31, 2024

  • Some tests have this backwards, probably reading "skip for ICU …". However, that is not how skip reasons are handled.
  • Some test seems to have typos in the skip reason version.
  • Some tests checked for a certain version, but reported the presumably next version, which is confusing at best.
  • Some tests checked versions in descending order, what is not wrong, but confusing.
  • Some tests had off by one errors.

For all tests, we assume that the skipif conditions are correct, and fix the reasons.


Note that I made this in preparation for #16659, to not completely loose my sanity.

* Some tests have this backwards, probably reading "skip for ICU …".
  However, that is not how skip reasons are handled.
* Some test seems to have typos in the skip reason version.
* Some tests checked for a certain version, but reported the
  *presumably* next version, which is confusing at best.
* Some tests checked versions in descending order, what is not wrong,
  but confusing.
* Some tests had off by one errors.

For all tests, we assume that the skipif conditions are correct, and
fix the reasons.
Copy link
Member

@devnexen devnexen left a comment

Choose a reason for hiding this comment

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

Thanks !

@cmb69 cmb69 merged commit 9afc66f into php:master Nov 2, 2024
8 of 10 checks passed
@cmb69 cmb69 deleted the cmb/icu-versions branch November 2, 2024 14:49
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