-
Notifications
You must be signed in to change notification settings - Fork 379
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
CLDR-17088 Flesh out en.xml #4110
Conversation
-New TestEnInheritance.java checks modern coverage inherited values in en that are marked with up arrows or are blank -Make TestPaths.extraPathAllowsNullValue public and static for use by TestEnInheritance -New CLDRModify option -fE corrects problems that would fail TestEnInheritance -Revise common/main/en.xml per running CLDRModify -fE
Please note: I just made this ready for review. However it's targeted for v47 not v46, and I'm not sure where we stand with the main branch and v46 vs v47. This PR is not urgent. |
open for merges. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really mean isBlank in the sense of Java, that is, consists of 0 or more whitespace characters?
The first comment in the ticket includes "Add a test to check modern coverage inherited values in en that are marked with up arrows or are blank". I don't know whether the intended meaning of "blank" matches the Java method. I reasoned that a broad interpretation would bring up any questionable values, and the method could be revised accordingly as needed. My first draft PR (#3444) implemented the test with isBlank, and you commented "There are a few values that can be blank, so they would need to be excluded." In response, I added exceptions for DisplayAndInputProcessor.FSR_START_PATH/NSR_START_PATH. The result is that no "blank" values are currently found by the test. (BTW this PR does change ↑↑↑ to space for FSR_START_PATH.) We could remove the code changes in this PR that involve isBlank, and I think it would make no difference to the data changes or the test results for this PR. After future changes to en.xml, the potential consequences for future runs of the test and the modify -fE tool aren't clear to me. Please let me know whether further changes are appropriate. |
I see what the problem is. Comment https://unicode-org.atlassian.net/browse/CLDR-17088?focusedCommentId=173120 says:
It should be: Add a test to check for modern coverage paths in en.xml that are inherited (the value is either up arrows or null); all such paths need to have explicit values in en.xml. This includes all directories with inheritance, so main/, annotations/, and annotationsDerived. |
The second commit removes the code changes involving "blank" values |
-New TestEnInheritance.java checks modern coverage inherited values in en that are marked with up arrows or are null
-Make TestPaths.extraPathAllowsNullValue public and static for use by TestEnInheritance
-New CLDRModify option -fE corrects problems that would fail TestEnInheritance
-Revise common/main/en.xml per running CLDRModify -fE
CLDR-17088
ALLOW_MANY_COMMITS=true