-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
feat: add scoringMode to AuditResult #1967
Conversation
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. Feels like this should also add documentation somewhere, but we don't really do that for the other meta
properties either
@@ -19,6 +19,7 @@ interface AuditFullResult { | |||
displayValue: string; | |||
debugString?: string; | |||
score: boolean|number; |
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.
Should add it to the closure typedef too
how do you feel about |
7d27e4b
to
5c36aa4
Compare
5c36aa4
to
3414327
Compare
@@ -45,6 +45,7 @@ class TotalByteWeight extends Audit { | |||
'Network transfer size [costs users real dollars](https://whatdoesmysitecost.com/) ' + | |||
'and is [highly correlated](http://httparchive.org/interesting.php#onLoad) with long load times. ' + | |||
'Try to find ways to reduce the size of required files.', | |||
scoringMode: 'numeric', |
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.
should we use an enum? scoringMode: Audit.ScoringModes.Numeric
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.
done
Double LGTM |
lighthouse-core/audits/audit.js
Outdated
*/ | ||
static get ScoringModes() { | ||
return { | ||
Numeric: 'numeric', |
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.
should be NUMERIC
and BINARY
or numeric
and binary
IMO. also SCORING_MODES
?
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.
done
0737440
to
7616527
Compare
Triple LGTM |
lol u guys and the yelling. lgtm |
No description provided.