-
Notifications
You must be signed in to change notification settings - Fork 9.5k
core(report-generator): add url to CSV output #10656
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
Conversation
const table = Object.values(lhr.categories).map(category => { | ||
return category.auditRefs.map(auditRef => { | ||
const audit = lhr.audits[auditRef.id]; | ||
// CSV validator wants all scores to be numeric, use -1 for now | ||
const numericScore = audit.score === null ? -1 : audit.score; | ||
return [category.title, audit.id, audit.title, audit.scoreDisplayMode, numericScore] | ||
return [lhr.requestedUrl, lhr.finalUrl, category.title, audit.id, audit.title, | ||
audit.scoreDisplayMode, numericScore] |
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.
we gotta remove the push action. it doubles up on the lint errors
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.
Pushing back against CSV as a thing is more work than making it better :)
yeah, we should just encourage someone to make an lhr -> csv
tool and put it up on npm somewhere :)
const table = Object.values(lhr.categories).map(category => { | ||
return category.auditRefs.map(auditRef => { | ||
const audit = lhr.audits[auditRef.id]; | ||
// CSV validator wants all scores to be numeric, use -1 for now | ||
const numericScore = audit.score === null ? -1 : audit.score; | ||
return [category.title, audit.id, audit.title, audit.scoreDisplayMode, numericScore] | ||
return [lhr.requestedUrl, lhr.finalUrl, category.title, audit.id, audit.title, |
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.
wait, so is there going to be two urls per line in the CSV?
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.
wait, so is there going to be two urls per line in the CSV?
I can live with that and don't care, just to be clear :)
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.
correct there will be :)
const table = Object.values(lhr.categories).map(category => { | ||
return category.auditRefs.map(auditRef => { | ||
const audit = lhr.audits[auditRef.id]; | ||
// CSV validator wants all scores to be numeric, use -1 for now | ||
const numericScore = audit.score === null ? -1 : audit.score; | ||
return [category.title, audit.id, audit.title, audit.scoreDisplayMode, numericScore] | ||
return [lhr.requestedUrl, lhr.finalUrl, category.title, audit.id, audit.title, | ||
audit.scoreDisplayMode, numericScore] |
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 respectfully disagree
yeah, I think that's an eslintrc bug
Summary
Pushing back against CSV as a thing is more work than making it better :)
Related Issues/PRs
closes #10654