-
Notifications
You must be signed in to change notification settings - Fork 382
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-17560 UI for entire locale checks #3644
CLDR-17560 UI for entire locale checks #3644
Conversation
With this change, there are no longer thousands of errors (compare to https://cldr-staging.unicode.org/cldr-apps/v#/vmw/Languages_A_D/7d1d3cbd260601a4 ) |
tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckCLDR.java
Outdated
Show resolved
Hide resolved
tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckCLDR.java
Outdated
Show resolved
Hide resolved
tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckCLDR.java
Outdated
Show resolved
Hide resolved
tools/cldr-code/src/main/java/org/unicode/cldr/tool/ChartSupplemental.java
Outdated
Show resolved
Hide resolved
Looks really useful. Can you add a copy of the resulting report, for comparison. |
Also, I would probably split the web portion of this out separately |
Agreed.
…On Thu, Apr 18, 2024 at 3:00 PM Steven R. Loomis ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckCLDR.java
<#3644 (comment)>:
> @@ -1117,6 +1119,44 @@ public static boolean hasType(List<CheckStatus> result, Type type) {
}
return false;
}
+
+ /**
+ * @returns true if this status applies to the entire locale, not a single path
+ */
+ public boolean isNonPathBased() {
The report name might also want to be something similar… Overall problems,
since *supplemental* will not mean anything to normal people :)
—
Reply to this email directly, view it on GitHub
<#3644 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACJLEMESPNJNW3C5N7U2YZDY6A67JAVCNFSM6AAAAABGOCNIUOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMBZHE4TCMZUGE>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Thanks… There is a screenshot included above |
ffff603
to
277e98c
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
I'm going to keep this PR for the UI and make a separate one for underpinnings |
5db570b
to
be946ad
Compare
Hooray! The files in the branch are the same across the force-push. 😃 ~ Your Friendly Jira-GitHub PR Checker Bot |
be946ad
to
dde975c
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
|
269db13
to
4368d82
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
4368d82
to
3925491
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
3925491
to
7eedaac
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
Still has conflicts. Otherwise ok |
7eedaac
to
7774140
Compare
Hooray! The files in the branch are the same across the force-push. 😃 ~ Your Friendly Jira-GitHub PR Checker Bot |
- add a new 'supplemental' report that simply lists the overall errors - update Chart API to take testbundle and subtype mapper
- rollup non-path checks into a single warning in the info panel - fix Voting API to expose this data
7774140
to
466bdcb
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
@macchiati take a look. |
@@ -752,8 +752,15 @@ function testsToHtml(tests) { | |||
if (!tests) { | |||
return newHtml; | |||
} | |||
var hadEntireLocale = false; |
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.
I never use "var" anymore. I don't know of any good reason to use it in new code. Not that it generally causes any problem, but it has an "old code" smell and raises questions like "is this variable referenced before it's declared?" and "is it declared more than once?"
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.
Will fix in follow on
(PR depends on #3648 )
CLDR-17560
ALLOW_MANY_COMMITS=true