-
Notifications
You must be signed in to change notification settings - Fork 389
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-17544 In en, remove yMd in chinese calendar availableFormats #3696
CLDR-17544 In en, remove yMd in chinese calendar availableFormats #3696
Conversation
…rious and (b) can't be reproduced locally.
Please review |
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.
LGTM though suggestions for improvement
if (path.contains("yMd") && path.contains("chinese")) { | ||
int debug = 0; | ||
} |
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.
if (path.contains("yMd") && path.contains("chinese")) { | |
int debug = 0; | |
} |
you should be able to do these with a conditional breakpoint, anyway
if (rootUncovered) { | ||
if (stringValue != null) { | ||
// Record any cases where there are non-null, non-inherited values | ||
if (!stringValue.equals("↑↑↑")) { |
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.
if (!stringValue.equals("↑↑↑")) { | |
if (!stringValue.equals(CldrUtility.INHERITANCE_MARKER)) { |
for (Iterator<String> it = file.iterator(); it.hasNext(); ) { | ||
String path = it.next(); | ||
if (extraPaths.contains(path)) { | ||
if (isExemptLocale && path.equals(exemptPathIfLocale)) { | ||
logKnownIssue("CLDR-17544", "Can't reproduce locally"); |
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.
^ the message isn't clear as to what's happening.
I'll make a followup PR with the tool improvements, thanks. |
logKnownIssue("CLDR-17544", "Can't reproduce locally"); | ||
continue; |
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.
Post script: This should have been if(logKnownIssue(…)) continue;
- only continue if logKnownIssue returns true.
CLDR-17544
This removes a value from root that isn't really needed, and fixes the example cited in the ticket.
It also adds an option to SearchCLDR to locate these kinds of issues, but doesn't address them all.
For that I'll clone the ticket.
ALLOW_MANY_COMMITS=true