-
Notifications
You must be signed in to change notification settings - Fork 9.5k
core(scoring): apply v6 score weightings #9949
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
Changes from all commits
a1126fc
033a06b
516ccf2
2d3e8f6
a5097a5
b85f9e2
f9fc5b3
17ba25b
5694740
035e4eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -179,8 +179,8 @@ const defaultConfig = { | |
'viewport', | ||
'without-javascript', | ||
'metrics/first-contentful-paint', | ||
'metrics/first-meaningful-paint', | ||
'metrics/largest-contentful-paint', | ||
'metrics/first-meaningful-paint', | ||
'load-fast-enough-for-pwa', | ||
'metrics/speed-index', | ||
'screenshot-thumbnails', | ||
|
@@ -388,16 +388,18 @@ const defaultConfig = { | |
'performance': { | ||
title: str_(UIStrings.performanceCategoryTitle), | ||
auditRefs: [ | ||
{id: 'first-contentful-paint', weight: 3, group: 'metrics'}, | ||
{id: 'first-meaningful-paint', weight: 1, group: 'metrics'}, | ||
{id: 'largest-contentful-paint', weight: 0, group: 'metrics'}, | ||
{id: 'speed-index', weight: 4, group: 'metrics'}, | ||
{id: 'interactive', weight: 5, group: 'metrics'}, | ||
{id: 'first-cpu-idle', weight: 2, group: 'metrics'}, | ||
{id: 'max-potential-fid', weight: 0, group: 'metrics'}, | ||
{id: 'cumulative-layout-shift', weight: 0}, // intentionally left out of metrics so it won't be displayed | ||
{id: 'estimated-input-latency', weight: 0}, // intentionally left out of metrics so it won't be displayed | ||
{id: 'total-blocking-time', weight: 0}, // intentionally left out of metrics so it won't be displayed | ||
{id: 'first-contentful-paint', weight: 15, group: 'metrics'}, | ||
{id: 'speed-index', weight: 15, group: 'metrics'}, | ||
{id: 'largest-contentful-paint', weight: 25, group: 'metrics'}, | ||
{id: 'interactive', weight: 15, group: 'metrics'}, | ||
{id: 'total-blocking-time', weight: 25, group: 'metrics'}, | ||
{id: 'cumulative-layout-shift', weight: 5, group: 'metrics'}, | ||
// intentionally left out of metrics group so they won't be displayed | ||
{id: 'first-cpu-idle', weight: 0}, | ||
{id: 'max-potential-fid', weight: 0}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so this is being killed afterall? I thought we brought up in a recent LH meeting that this was going to stay even if it wasn't weighted but the discussion in the doc suggests this is gone gone. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's still in the JSON but not visible anymore. We've long wanted to add Top Long Tasks as a diagnostic and I think that will end up being more helpful than keeping the mpFid metric. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
right, but with the intention of removing from the JSON in the future? I know the support was overwhelming for killing in the doc discussion but I still think it's going to be really unfortunate to have no FID-like equivalent for lab when it's like half our performance story in field. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aside: i wouldn't want to remove this from json. but further I'd prefer a average FID metric (instead of just a max one - actually this is just EIL) |
||
{id: 'first-meaningful-paint', weight: 0}, | ||
{id: 'estimated-input-latency', weight: 0}, | ||
|
||
{id: 'render-blocking-resources', weight: 0, group: 'load-opportunities'}, | ||
{id: 'uses-responsive-images', weight: 0, group: 'load-opportunities'}, | ||
{id: 'offscreen-images', weight: 0, group: 'load-opportunities'}, | ||
|
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.
are we generally still going to try to stick to the "ship in a release without weight" before we can ship in a breaking change with weight in the future/do we still plan to do that here with cherry-picking? or is this our new policy moving forward?
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.
Generally, the ideal flow for introducing a new metric is:
while we didnt get to follow this pattern here, i'd like to use that plan going forward. hows that sound?
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.
great! yeah agreed.
I still think we can follow part of that pattern here if we wanted to cherry-pick those metrics into a 5.7.0 or something, so folks can play with them without jumping to a new major. I guess that was the root of my question that just was leading in an unintended direction :)