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

core(scoring): update CLS score curve - 0.25 is now failing #10495

Merged
merged 5 commits into from
Apr 28, 2020

Conversation

paulirish
Copy link
Member

@paulirish paulirish commented Mar 23, 2020

keeping up with the joneses.

CLS of 0.25 and above is now considered failing
tweak the PODR to make sure CLS 0.10 is still a score of 90/100

https://goto.google.com/tbsgg

@paulirish paulirish requested a review from a team as a code owner March 23, 2020 19:05
@paulirish paulirish requested review from patrickhulce and removed request for a team March 23, 2020 19:05
@brendankenny
Copy link
Member

lol how does this file have no tests

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

LGTM % link update

https://goto.google.com/tbsgg

I'm not sure what's here, so if it matters to the review maybe get a googler's eyes too?

lol how does this file have no tests

FWIW the sample JSON artifact serves the same function as what I imagine would be the lone unit test

// see https://www.desmos.com/calculator/wmcxn7zfhc
scorePODR: 0.02,
scoreMedian: 0.2,
// see https://www.desmos.com/calculator/vjb652sqeh
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems like it didn't save with the new values, maybe just forgot to press save?

Copy link
Member Author

Choose a reason for hiding this comment

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

image

true. just updated with the newer shiny one.

@@ -46,9 +46,9 @@ class CumulativeLayoutShift extends Audit {
return {
// Calibrated to assure 0.1 gets a score of 0.9. https://web.dev/cls/#what-is-a-good-cls-score
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should really consider reworking our math so we can define in terms of the 90 cutoff :) haha

@brendankenny
Copy link
Member

I'm not sure what's here, so if it matters to the review maybe get a googler's eyes too?

it's just a doc of thresholds used for field data too

lol how does this file have no tests

FWIW the sample JSON artifact serves the same function as what I imagine would be the lone unit test

substitute "this file" with "these files" to include the computed artifact too :)

@patrickhulce
Copy link
Collaborator

conflicts though :)

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

Successfully merging this pull request may close these issues.

5 participants