-
Notifications
You must be signed in to change notification settings - Fork 384
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-17680 fix spurious CheckConsistentCasing and CheckCoverage #3767
Conversation
6834a40
to
ec146d2
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
- CheckConsistentCasing was giving a warning for scripts that don't need casing - CheckCoverage was giving an warning when plurals were == root, rather than a warning when plurals were missing
ec146d2
to
3875f8b
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
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.
It is hard to tell whether this is removing the errors from the main page; it doesn't appear that it does.
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.
Based on your email, this doesn't remove the messages from the main pages.
if ((types == null || types.isEmpty()) && !SpecialLocales.isScratchLocale(locale)) { | ||
possibleErrors.add( | ||
new CheckStatus() | ||
.setCause(this) | ||
.setMainType(CheckStatus.warningType) | ||
.setSubtype(Subtype.incorrectCasing) | ||
.setMessage("Could not load casing info for {0}", locale)); | ||
} |
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.
Previously, this warning would fire for Arabic, etc. because the script is not cased, so control would reach line 70. Then types is checked again on this line and the warning fires.
The change moves the warning to line 67, so it fires only if casing is expected but not present.
PluralInfo pluralInfo = | ||
supplementalData.getPlurals(PluralType.cardinal, localeID, false); | ||
if (pluralInfo == null && !SpecialLocales.isScratchLocale(localeID)) { |
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.
Previously, the warning would always fire for Japanese, etc, because the plural rules matched root. This changes to only fire if plural rules were not found.
It now does |
I thought issue was just where it was incorrect. It now should remove from all |
CLDR-17680
ALLOW_MANY_COMMITS=true