CLDR-18574 Fix coverage for iso8601 #4653
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
FYI, with the debug flag on, the unit test shows the levels that result. The comprehensive ones will not show up in the survey tool. They will be be the same as Gregorian. They are included here so that we can verify that we don't want any of them. The ones we do want are date (or date interval) skeletons with more than one field. These are the ones that have ordering issues. The ones we don't want are:
|
|
I changed the printout so that reviewers could see more clearly which of the items are being excluded. |
So that reviewers could see which items are excluded (comprehensive) or not (moderate/modern).
To show what happens with more values.
srl295
approved these changes
May 2, 2025
| // drop IDs unless they have y+M or M+d | ||
| List<String> raw = | ||
| Splitters.VBAR.splitToList( | ||
| "MMMMd|MMMMW|MMMMW|yw|yw|yQQQ|yQQQQ|Gy|Gy|GyM|GyM|GyM|GyMd|GyMd|GyMd|GyMd|GyMEd|GyMEd|GyMEd|GyMEd|GyMMM|GyMMM|GyMMM|GyMMMd|GyMMMd|GyMMMd|GyMMMd|GyMMMEd|GyMMMEd|GyMMMEd|GyMMMEd|Md|Md|MEd|MEd|MMMd|MMMd|MMMEd|MMMEd|Ed|yM|yM|yMd|yMd|yMd|yMEd|yMEd|yMEd|yMMM|yMMM|yMMMd|yMMMd|yMMMd|yMMMEd|yMMMEd|yMMMEd|yMMMM|yMMMM"); |
There was a problem hiding this comment.
Could this be generated from 'those without y+M or M+d'? seems fragile to have yet another static list.
There was a problem hiding this comment.
This is just for testing.
| } | ||
| } | ||
| } | ||
| if (Level.CORE_TO_MODERN.contains(level) == expectedComprehensive) { |
There was a problem hiding this comment.
Suggested change
| if (Level.CORE_TO_MODERN.contains(level) == expectedComprehensive) { | |
| if (!level.isAbove(MODERN) == expectedComprehensive) { |
There was a problem hiding this comment.
Good suggestion but I didn't want to wait on getting PR in.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CLDR-18574
This narrows the coverage of iso8601 to only date patterns. For available and interval formats, that is tested by making sure that the
idvalue matches a regular expression that contains 2 "date" pattern characters. That uses the line:The regex has the optional non " characters before and after, and sees if there are certain pairs, like Gy or Md. That is, it excludes singleton date characters like value='d', because ordering does not matter for them.
Adds a test, and a couple of GP utility classes.
ALLOW_MANY_COMMITS=true