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

[tests] correct high/low naming of UTF-16 surrogate character test fixture constants #1326

Closed
wants to merge 1 commit into from

Conversation

IBue
Copy link
Contributor

@IBue IBue commented Dec 3, 2024

The naming of the UTF-16 surrogate character test fixtures is semantically interchanged regarding "high" and "low".
This pull request corrects and clarifies the naming and also composes the supplementary character fixtures of the surrogate character test fixture constants where applicable.

…surrogate character test fixture constants;
compose supplementary characters of declared surrogates where applicable

(cf. 1f06eb4)
@garydgregory
Copy link
Member

Hello @IBue

Good catch! TY.

The issue with this PR IMO is that it does more than addressing the name issue. Changing the tests makes reviewing and checking if something went wrong harder. For example, it's unclear if you are also fixing something in the test. If you update your git master branch, you'll see I took a simpler approach and only changed the 2 constant names and updated the Javadoc.

You are credited in changes.xml.

@IBue
Copy link
Contributor Author

IBue commented Dec 4, 2024

Thank you for your quick review and taking this into account.
Well, but actually, I still find my patches easier to synoptically check for — as intended — factual test identity than your rework (especially in split view)…

closing in favour of f0c370e

@IBue IBue closed this Dec 5, 2024
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.

2 participants