Skip to content
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

Resolve audit result #487

Merged
merged 3 commits into from
Jul 8, 2016
Merged

Conversation

wardpeet
Copy link
Collaborator

Should fix #458
No idea if I did any good but basically this is what i've done.

Every value is mapped to rawValue (so value => rawValue).
Score is equal to score, if no score is set we fallback to rawValue
display value uses rawvalue with minor adaptation like adding ms or fallbacks to string representation of rawValue. If displayValue === score we make it empty so we don't display it in our reporting

throw new Error('generateAuditResult requires a rawValue');
}

let score = result.score ? result.score : result.rawValue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably use an undefined check as to not discard possible scores of 0

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct :)

@wardpeet wardpeet force-pushed the resolve-audit-result branch from 422cfd1 to 53fa9c5 Compare July 2, 2016 21:05
@wardpeet
Copy link
Collaborator Author

wardpeet commented Jul 2, 2016

Updated :)

@paullewis
Copy link
Contributor

Looks like it fails tests :(

@wardpeet wardpeet force-pushed the resolve-audit-result branch from 53fa9c5 to 3ebfdba Compare July 6, 2016 10:27
@wardpeet
Copy link
Collaborator Author

wardpeet commented Jul 6, 2016

Forgot to push :'( waiting for the build :)

@wardpeet wardpeet force-pushed the resolve-audit-result branch from 3ebfdba to 54a0ff2 Compare July 6, 2016 11:59
@paulirish paulirish merged commit 54a0ff2 into GoogleChrome:master Jul 8, 2016
@paulirish
Copy link
Member

awesome stuff. A massive cleanup. Thanks for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resolve audit result values
3 participants