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

docs: update docs for v3 #5357

Merged
merged 2 commits into from
May 25, 2018
Merged

docs: update docs for v3 #5357

merged 2 commits into from
May 25, 2018

Conversation

patrickhulce
Copy link
Collaborator

Summary
updates lots of docs for v3 changes

Related Issues/PRs
closes #5355
closes #3587

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!

Nice work slogging through these

@@ -84,6 +84,6 @@ Tracing processor takes the output of trace of tab and identifies the top-level

## Audits

The return value of each audit [takes this shape](https://github.com/GoogleChrome/lighthouse/blob/b354890076f2c077c5460b2fa56ded546cca72ee/lighthouse-core/closure/typedefs/AuditResult.js#L23-L55).
The return value of each audit [takes this shape](https://github.com/GoogleChrome/lighthouse/blob/8f500e00243e07ef0a80b39334bedcc8ddc8d3d0/typings/audit.d.ts#L117-L130).
Copy link
Member

Choose a reason for hiding this comment

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

nice catch, didn't even think to search for these.... :(

Choose a reason for hiding this comment

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

hi

port: (new URL(browser.wsEndpoint())).port,
output: 'json',
logLevel: 'info',
});
})).lhr;
Copy link
Member

Choose a reason for hiding this comment

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

would destructuring style const {lhr} = await lighthouse... read slightly better?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ooo yeah much better 👍

| `configPath` | Ignored, pass the config in as the 3rd argument to `lighthouse`. |
| `preset` | Ignored, pass the config in as the 3rd argument to `lighthouse`. |
| `verbose` | Ignored, use `logLevel` instead. |
| `quiet` | Ignored, use `logLevel` instead. |
Copy link
Member

Choose a reason for hiding this comment

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

this is a great new section

docs/readme.md Outdated
```

You can also craft your own config (e.g. [mixed-content-config.js](https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/config/mixed-content-config.js)) for custom runs. Also see the [basic custom audit recipe](https://github.com/GoogleChrome/lighthouse/tree/master/docs/recipes/custom-audit).

### A warning about flags
Copy link
Member

Choose a reason for hiding this comment

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

maybe a less ominous title for the section? :) Just ### CLI flags that have no effect as module options or something better than that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

went with "Differences from CLI flags"

docs/scoring.md Outdated
@@ -6,7 +6,7 @@ Note 1: if you want a **nice spreadsheet** version of this doc to understand wei
![alt text](https://user-images.githubusercontent.com/39191/32397461-2d20c87a-c0a7-11e7-99d8-61576113a710.png)
*Screenshot of the [scoring spreadsheet](https://docs.google.com/spreadsheets/d/1Cxzhy5ecqJCucdf1M0iOzM8mIxNc7mmx107o5nj38Eo/edit#gid=0)*

Note 2: if you receive a **score of 0** in any Lighthouse category, that usually indicates an error on our part. Please file an [issue](https://github.com/GoogleChrome/lighthouse/issues) so our team can look into it.
Note 2: if you receive a **score of ?** in any Lighthouse category, that indicates an error. Please file an [issue](https://github.com/GoogleChrome/lighthouse/issues) so our team can look into it.
Copy link
Member

Choose a reason for hiding this comment

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

indicates an error occurred, maybe?


## Throttling basics

1. The DevTools network throttling, which Lighthouse uses, is implemented within Chrome at the _request-level_. As a result, it has some downsides that are now summarized in this doc: [Network Throttling & Chrome - status](https://docs.google.com/document/d/1TwWLaLAfnBfbk5_ZzpGXegPapCIfyzT4MWuZgspKUAQ/edit). The TLDR: while it's a [decent approximation](https://docs.google.com/document/d/1uS9SH1KpVH31JAmf-iIZ61VazwAF9MrCVwETshBC4UQ/edit), it's not a sufficient model of a slow connection. The [multipliers used in Lighthouse](https://github.com/GoogleChrome/lighthouse/blob/3be483287a530fb560c843b7299ef9cfe91ce1cc/lighthouse-core/lib/emulation.js#L33-L39) attempt to correct for the differences.
1. Simulated throttling, which Lighthouse uses by default, is computed throttling _after a trace as been recorded_ which makes it very fast and deterministic. However, due to the imperfect nature of predicting alternate execution paths, there is inherrent inaccuracy that is summarized in this doc: [Lighthouse Metric Variability and Accuracy](https://docs.google.com/document/d/1BqtL-nG53rxWOI5RO0pItSRPowZVnYJ_gBEQCJ5EeUE/edit). The TLDR: while it's roughly as accurate or better than DevTools throttling for most sites, it suffers from edge cases and a deep investigation to performance should use _Packet-level_ throttling tools.
Copy link
Member

Choose a reason for hiding this comment

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

as been -> has been
inherrent -> inherent

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍

## import your report renderer into devtools-frontend and run devtools closure compiler
yarn compile-devtools
## run tsc compiler
yarn type-check
Copy link
Member

Choose a reason for hiding this comment

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

mind also updating the mention of "closure annotations" in CONTRIBUTING.md too? Can link to https://github.com/Microsoft/TypeScript/wiki/JSDoc-support-in-JavaScript instead of that closure link

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@patrickhulce patrickhulce merged commit e410b34 into master May 25, 2018
@patrickhulce patrickhulce deleted the docs_updates branch May 25, 2018 21:55
@@ -84,6 +84,6 @@ Tracing processor takes the output of trace of tab and identifies the top-level

## Audits

The return value of each audit [takes this shape](https://github.com/GoogleChrome/lighthouse/blob/b354890076f2c077c5460b2fa56ded546cca72ee/lighthouse-core/closure/typedefs/AuditResult.js#L23-L55).

Choose a reason for hiding this comment

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

download 6

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

Successfully merging this pull request may close these issues.

Document dependency on node 8 LTS Document which CLI flags are/aren't available from node module
4 participants