CLDR-17560 Overall Errors #3743
Conversation
- add new CldrError and OverallErrors components - add new REST endpoint for whole-locale-errors - remove Report for supplemental data (whole locale errors) and references thereto - remove link from info panel to supplemental chart - add whole-locale widget to General Info
srl295
force-pushed
the
cldr-17560/overall-errors
branch
from
4299f78 to
279ce0f
Compare
|
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
|
No output if there's no issue. Otherwise: 
|
|
this is up on staging as of this writing |
| @@ -788,11 +788,6 @@ function testsToHtml(tests) { | |||
|
|
|||
| newHtml += "</p>\n"; | |||
| } | |||
| if (hadEntireLocale) { | |||
There was a problem hiding this comment.
this makes hadEntireLocale unused (dead code), so it should be removed above
|
|
||
| <script> | ||
| // import * as cldrLoad from "../esm/cldrLoad.mjs"; | ||
| // import * as cldrStatus from "../esm/cldrStatus.mjs"; |
There was a problem hiding this comment.
commented-out code should be removed
| subtypeString() { | ||
| return this.status.subtype | ||
| .split(/(?=[A-Z])/) | ||
| .map((s) => s) |
There was a problem hiding this comment.
I don't understand, what is the need for .map((s) => s)? Wouldn't the code do the same if that were removed?
There was a problem hiding this comment.
this was originally a lowercasing function. There's no titlecase in JS.
| status: { | ||
| type: Object, | ||
| // any additional classes | ||
| default: "", |
There was a problem hiding this comment.
should message, subtype and subtypeUrl be declared here? It looks like they're filled in from json from the back end. If that fails for any reason, the front end will throw cryptic errors like undefined subtype... I know we already have a lot of js code that makes this kind of implicit assumption. Still, objects with clearly defined and documented properties would be more ideal. I think status corresponds to back-end CheckCLDR.CheckStatus, and subtype corresponds to CheckCLDR.CheckStatus.Subtype. Comments could help...
There was a problem hiding this comment.
I don't think it will type check here.
| }, | ||
| props: { | ||
| status: { | ||
| type: Object, |
There was a problem hiding this comment.
is type an object, or a string? On the back end it's an enum, but in the json it looks very much like a string such as "Warning" -- or am I just totally confused?
There was a problem hiding this comment.
It's an object. In fact it's a serialized CheckStatusSummary.
There was a problem hiding this comment.
I don't think we're talking about the same thing... status itself is an object, yes (I wrongly guessed CheckCLDR.CheckStatus but you say CheckStatusSummary and I believe you).
I was referring to status.type. On the back end that's CheckCLDR.CheckStatus.Type, which is an enum, and on the front end it's not an Object, it's a String. So, I think type: Object is wrong, it should be type: String
There was a problem hiding this comment.
oh, type: here is part of Vue, it's not status.type. See https://vuejs.org/guide/components/props.html compare to:
propE: {
type: Number:
default: 100,
},There was a problem hiding this comment.
I could have written:
props: {
status: Object
}
|
do we want to merge changes to https://unicode-org.atlassian.net/browse/CLDR-17664 before merging this? |
|
@btangmu i'm going to make this one draft because it's waiting on some further UI review |
- per Code Review
- in #3743, ReportId.supplemental was retired, but it occurs in some user rows - skip over invalid rows
- in #3743, ReportId.supplemental was retired, but it occurs in some user rows - skip over invalid rows
CLDR-17560
This PR completes the ticket.
add new CldrError and OverallErrors components
add new REST endpoint for whole-locale-errors
remove Report for supplemental data (whole locale errors) and references thereto
remove link from info panel to supplemental chart
add whole-locale widget to General Info
ALLOW_MANY_COMMITS=true