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(audits): point perf audit description links to web.dev #9540

Merged
merged 3 commits into from Sep 27, 2019
Merged

core(audits): point perf audit description links to web.dev #9540

merged 3 commits into from Sep 27, 2019

Conversation

mfriesenhahn
Copy link
Collaborator

Summary
Points "Learn more" links in performance audits to relevant web.dev guides.

Note
web.dev currently doesn't have audit-oriented posts for the Keep request counts low and transfer sizes small and Third-party Usage audits. However, there are posts in the Learn section whose content could be repurposed. We're working to figure out how quickly that can be done.

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.

alright heads up in advance this section hits close to home for me, so I'm a bit picky here :)

Note that lots of this is pre-existing and I'm only now bringing up because we're moving to a new docs system with a chance to make some big improvements.

Some thoughts:

  • The metrics docs are especially important to get right IMO and I'm a bit worried about using screenshots from a major version behind that all the reports point to. For example, we've removed EIL and replaced it with mpFID (which needs a doc) in 5.0 so the screenshots are already out of date the day they go live with more metric changes planned in the next 2 months. It seems like it would be a challenge to try to keep these up to date and even if we could the fact that they all point to the same doc means everyone in the old version will be looking at something different. Maybe this is the one area we would want to version our docs?
  • The score table and explanation for several of the metrics is a bit off. It looks like the values for green/red cutoffs were taken from the median and PODR but that's not exactly what they map to. It's correct that the median is the cutoff for orange/red but the PODR is more like a ~98 and the cutoff for green since 4.0 is a score of 90, not 75.
  • In some of the metric score tables it also expresses that the score is the HTTPArchive %-ile but that's a little off. We derive the score curve from the httparchive percentiles, yes, but the 75th percentile in httparchive is a 50 in Lighthouse and the 95th percentile in httparchive is a ~98. It might be tricky or at least confusing to our users to try to present this exact curve mapping.
  • Bootup time and mainthread breakdown are some of the most important audits that can't tell users to do anything about automatically. This to me is a big important place to deliver on guides and link to topics like bundle splitting, alternatives to beefy libraries and third-parties, etc. I would love to see these fleshed out.
  • Defer offscreen images in an important and common topic that comes up a lot, I would personally prioritize fleshing this doc out a bit more.
  • dom-size audit doesn't have a new link, is that expected?

Copy link
Member

@exterkamp exterkamp left a comment

Choose a reason for hiding this comment

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

I'm reviewing only from a code perspective, and it LGTM.

lighthouse-core/audits/metrics/first-cpu-idle.js Outdated Show resolved Hide resolved
@mfriesenhahn
Copy link
Collaborator Author

@patrickhulce, I've copied your comments to (web.dev issue 1344](GoogleChrome/web.dev#1344) for tracking.

@paulirish
Copy link
Member

i resolved the merge conflicts. we'll merge this PR whenever it's green.

naturally, i would like to see patrick's feedback addressed within the articles. :)

@mfriesenhahn
Copy link
Collaborator Author

naturally, i would like to see patrick's feedback addressed within the articles. :)

Patrick's feedback has been included in the web.dev issue (#1344) we're using to track the doc updates.

@brendankenny brendankenny changed the title core(audits): Point perf audit links to web.dev core(audits): point perf audit description links to web.dev Sep 27, 2019
Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

LGTM (and @paulirish's earlier comment looks like the same)

just waiting for tests to be green after #9737

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

Successfully merging this pull request may close these issues.

None yet

6 participants