README: Reorganization and new "Using programmatically" section #1721

ebidel · 2017-02-14T21:54:12Z

R: all

Also fixes #1719.

The example is extracted and simplified from lighthouse hue lights thing I made. Happy to rework it a little, but it's pretty chill and is known to be everything you need to call LH programmatically.

paulirish · 2017-02-14T22:02:42Z

readme.md


 The traceviewer-based trace processor from [node-big-rig](https://github.com/GoogleChrome/node-big-rig/tree/master/lib) was forked into Lighthouse. Additionally, the [DevTools' Timeline Model](https://github.com/paulirish/devtools-timeline-model) is available as well. There may be advantages for using one model over another.

+### Trace processing
+
 **To update traceviewer source:**


this is dev instructions for us, and belongs in the contributing.md

paulirish · 2017-02-14T22:03:21Z

readme.md

+DevTools](https://medium.com/@paul_irish/debugging-node-js-nightlies-with-chrome-devtools-7c4a1b95ae27#.59rma3ukm)
+for more info.
+
+## Coding Style


move to contributing.md

paulirish · 2017-02-14T22:03:37Z

readme.md

+
+# The CLI is authored in TypeScript and requires compilation.
+# If you need to make changes to the CLI, run the TS compiler in watch mode:
+# cd lighthouse-cli && npm run dev


link to contributing and architecture somewhere around here?

and can you link to architecture doc from contributing.md?

paulirish · 2017-02-14T22:04:05Z

readme.md

+
+### Are results sent to a remote server?
+
+Hope. Lighthouse runs locally, auditing a page using a local version of the Chrome browser installed the


paulirish · 2017-02-14T22:05:37Z

readme.md

 <p align="center">
-  <img src="https://cloud.githubusercontent.com/assets/39191/22478294/23f662f6-e79e-11e6-8de3-ffd7be7bf628.png" alt="Lighthouse logo" height="300">
+  <img src="https://cloud.githubusercontent.com/assets/39191/22478294/23f662f6-e79e-11e6-8de3-ffd7be7bf628.png" alt="Lighthouse logo" height="150">
  <br>
  <b>Lighthouse</b>, ˈlītˌhous (n): a <s>tower or other structure</s> tool containing a beacon light to warn or guide <s>ships</s> developers at "sea".


~~ships at sea~~ developers.

IMO slightly better. if you want.

paulirish · 2017-02-14T22:07:23Z

docs/architecture.md

@@ -0,0 +1,60 @@
+# Architecture


IMO this file can get merged with API-and-internals.md

and since readme and contributing are in root, i kinda prefer keeping this md file there rather than docs/. wdyt?

Up to you. I figured we'd have more in the future and wanted to declutter.

There's also https://help.github.com/articles/configuring-a-publishing-source-for-github-pages/#publishing-your-github-pages-site-from-a-docs-folder-on-your-master-branch, which we could leverage someday.

Moved and renamed that file to "headless-chrome.md". It didn't contain anything but headless Chrome stuff :)

patrickhulce

nice it was due for some cleanup!

patrickhulce · 2017-02-14T22:03:24Z

docs/architecture.md

+## Components
+
+* **Driver** - Interfaces with [Chrome Debugging Protocol](https://developer.chrome.com/devtools/docs/debugger-protocol)  ([API viewer](https://chromedevtools.github.io/debugger-protocol-viewer/))
+* **Gathers** - Requesting data from the browser (and maybe post-processing)


patrickhulce · 2017-02-14T22:04:54Z

docs/architecture.md

+* **Artifacts** - The output of gatherers
+* **Audits** - Non-performance evaluations of capabilities and issues. Includes a raw value and score of that value.
+* **Metrics** - Performance metrics summarizing the UX
+* **Diagnoses** - The perf problems that affect those metrics


Is this still a thing that we call Diagnoses?

patrickhulce · 2017-02-14T22:13:16Z

readme.md


-Some basic unit tests forked are in `/test` and run via mocha. ESLint also checks for style violations.
+The example below shows how to setup and run Lighthouse programmatically as a Node module.


How important is supporting use as a node module to us? If we think it's important, any issues to make basic usage easier than 80 lines? If not, maybe let's move this to separate file and link to it from README.

Personally, I'd love to see chrome launcher published to NPM separately with a slightly friendlier interface (happy to take this up if there's agreement), and a more organized programmatic API that doesn't involve so much digging into our directory tree (more resilient to doing our rename of lighthouse-cli -> cli etc

Open sourcing a more convenient launcher would be great. That's really the bulk of the example, and something I struggled to get working for my demo.

When looking into a lib, I do really like seeing install + different usages in the main readme. Since a lot of people ask for docs on this, I'd love to keep it front and center. Ironically, an email thread from Addy prompted me to add this to our radme...which then led to a full reorg :)

Happy to move it out though.

When looking into a lib, I do really like seeing install + different usages in the main readme.

Totally agree, I just also would find it off-putting if the usage example was so complicated :) I say let's leave it here, and I'll file issue to improve the API.

ebidel · 2017-02-14T23:40:21Z

PTAL

brendankenny · 2017-02-14T23:37:37Z

readme.md

+const PERF_CONFIG = require('lighthouse/lighthouse-core/config/perf.json');
+// const DEFAULT_CONFIG = require('lighthouse/lighthouse-core/config/default.json');
+
+const _SIGINT = 'SIGINT';


Doesn't this seem like overkill? Handling command line args and SIGINT seems out of scope for a readme example

brendankenny · 2017-02-14T23:39:21Z

readme.md

+  }
+
+  // Example of extracting the overall score from the results.
+  getOverallScore(results) {


Ha, seems like we should already be including this in the lighthouse results

Sure, but I don't think it hurts to show an "Example of extracting" things from our crazy config object.

kept, but moved it as a separate function example

ha, sorry, I meant it's ridiculous that the results object don't currently include the overall score and that it has to be extracted :) Somehow I hadn't noticed that in all this time. We should add it.

Gotcha. I was surprised too. But I guess we've also never come across it. We have the notion of scores for each aggregation, but only have 1 that's scored atm.

brendankenny · 2017-02-14T23:42:25Z

readme.md

+  }
+}
+
+const flags = yargs


yargs especially seems like it could be cut. 9 times out of 10 it seems like people will be calling lighthouse within an existing script or with hardcoded values

+1 to this.

you can see https://github.com/paulirish/pwmetrics/blob/27ee73fd1c9c8ed5457eebafb245b1592ac26a38/lib/index.js#L64-L90 which is how we adopt LH in pwmetrics. it's pretty small.

ebidel · 2017-02-15T03:27:15Z

Removed the extras from the example

paulirish · 2017-02-15T03:29:16Z

Yeah this small example is clutch. Nice!

ebidel · 2017-02-15T17:19:56Z

@patrickhulce how does this look?

patrickhulce

great improvements!

README: reorganize and add programmatic section

8e5e0c2

ebidel changed the title ~~README: reorganize and add programmatic section~~ README: Reorganization and new "Using programmatically" section Feb 14, 2017

paulirish requested changes Feb 14, 2017

View reviewed changes

patrickhulce suggested changes Feb 14, 2017

View reviewed changes

feedback

ad28577

patrickhulce mentioned this pull request Feb 14, 2017

Improve node programmatic API #1722

Closed

Move API-internals -> headless-chrome

556b9b7

brendankenny reviewed Feb 14, 2017

View reviewed changes

paulirish approved these changes Feb 15, 2017

View reviewed changes

smaller example

918d796

ebidel force-pushed the readme branch from 66eb2c0 to 918d796 Compare February 15, 2017 03:35

patrickhulce approved these changes Feb 15, 2017

View reviewed changes

ebidel merged commit c0d7ebf into master Feb 15, 2017

ebidel deleted the readme branch February 15, 2017 18:21


		### Are results sent to a remote server?

		Hope. Lighthouse runs locally, auditing a page using a local version of the Chrome browser installed the


		Some basic unit tests forked are in `/test` and run via mocha. ESLint also checks for style violations.
		The example below shows how to setup and run Lighthouse programmatically as a Node module.

README: Reorganization and new "Using programmatically" section #1721

README: Reorganization and new "Using programmatically" section #1721

Conversation

ebidel commented Feb 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebidel Feb 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrickhulce left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebidel Feb 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebidel commented Feb 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebidel commented Feb 15, 2017

paulirish commented Feb 15, 2017

ebidel commented Feb 15, 2017

patrickhulce left a comment

Choose a reason for hiding this comment

ebidel commented Feb 14, 2017 •

edited

Loading

ebidel Feb 14, 2017 •

edited

Loading

ebidel Feb 14, 2017 •

edited

Loading