-
Notifications
You must be signed in to change notification settings - Fork 9.6k
report: group perf audits by details type #13241
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
| !Util.showAsPassed(audit.result)); | ||
| const oppAudits = category.auditRefs.filter(audit => | ||
| audit.result.details && | ||
| audit.result.details.type === 'opportunity' && |
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.
if the logic for determining opportunity is used in many different files (I've only counted two so far in my review tho) would be worth moving to renderer util.
brendankenny
left a comment
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.
looks great! not sure the best place for it, but it would be good to document somewhere the special perf category handling, either in render or maybe on @connorjclark's suggested classifyPerformanceAudit function
…thouse into report-perf-auto-group
Closes #12828, allowing h2 audit in timespan mode.
No audits in the default config have group
load-opportunitiesordiagnosticsanymore. Havinggroupunset now means the renderer will automatically choose the group based on the audit details type.Couple more ideas:
load-opportunitiesordiagnostics, put in specified group without looking at result details. This means the renderer could still support legacy LHRs.hiddengroup, have anautogroup and revert to hiding audits with no group in perf.Ref #11313