-
Notifications
You must be signed in to change notification settings - Fork 386
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-15161 3rd meter: correct error count #1671
Conversation
- send in dpath (not fullpath) to check(), avoid spurious collision errors - also add some more debugging values to the response payload
@@ -142,25 +142,34 @@ static LocaleCompletionResponse getLocaleCompletion(final CLDRLocale cldrLocale, | |||
|
|||
final List<CheckStatus> results = new ArrayList<>(); | |||
for (final String xpath : file) { | |||
lcr.allXpaths ++; |
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.
unusual space before "++"
i wish we were using similar formatting tools for java as we're using for js
public int ignoredHidden = 0; | ||
public int allXpaths = 0; | ||
public int statusMissing = 0; | ||
public int ignoredOutOfCov = 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.
it might be worthwhile to make a stronger distinction between debug and non-debug items
is this data included in json, and if so, is it actually used/needed on the front end?
could these be private?
on the one hand, it can be convenient and flexible like this to be public (and maybe included in json), however in the long run there's a risk of cruft, dead code, and unnecessary dependencies
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.
Yes, they are included in the JSON, and so could be used in an expanded sort of meter with a breakout.
} // else: out of coverage level, do not count the path. | ||
} else { | ||
// out of coverage level, do not count the path. | ||
lcr.ignoredOutOfCov ++; |
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.
there's that space plus plus again
Merging so we can get eyes on the functionality.. will leave ticket open to revisit formatting (thanks) |
@@ -193,6 +205,13 @@ static LocaleCompletionResponse getLocaleCompletion(final CLDRLocale cldrLocale, | |||
public int provisional = 0; | |||
final public String level; | |||
|
|||
// The following are more or less debug items |
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.
not shown to user, but calculated as part of counts
public int ignoredHidden = 0; | ||
public int allXpaths = 0; | ||
public int statusMissing = 0; | ||
public int ignoredOutOfCov = 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.
Yes, they are included in the JSON, and so could be used in an expanded sort of meter with a breakout.
CLDR-15161