Prep export of launcher as a standalone module #2358

samccone · 2017-05-25T03:08:27Z

For now depend on lighthouse to use the logger. Eventually I vote that we pull out logger into a standalone module to consume to in both lighthouse and in launcher.

Fixes #2251
Fixes #2469.

Ref #2092

googlebot · 2017-05-25T03:08:29Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

denar90 · 2017-05-25T08:04:44Z

chrome-launcher/package.json

+    "@types/node": "6.0.66",
+    "lighthouse": "^2.0.0"
+  },
+  "version": "0.0.1",


buggy version lol

let's start with a 0.1.0 at very least.

patrickhulce · 2017-05-25T16:41:47Z

chrome-launcher/.npmignore

+node_modules/
+
+# include all compiled JS
+!*.js


what rule is this negating? forcing the inclusion of tests or something?

gitignore
https://docs.npmjs.com/misc/developers#keeping-files-out-of-your-package

the ignore files won't be merged though, is the parent folder ignore creeping in?

patrickhulce · 2017-05-25T16:42:09Z

chrome-launcher/.npmignore

+
+# dev files
+.appveyor.yml
+.DS_Store


nit: unnecessary (along with npm-debug.log and node_modules)

paulirish

We're gonna need a readme for this to be really real. :)

paulirish · 2017-05-26T23:38:23Z

chrome-launcher/package.json

+    "@types/node": "6.0.66",
+    "lighthouse": "^2.0.0"
+  },
+  "version": "0.0.1",


let's start with a 0.1.0 at very least.

denar90 · 2017-05-28T22:24:20Z

@samccone do you need some help with readme? I'm not so good at writing docs but I can try to help.

samccone · 2017-06-10T15:44:55Z

readme added, and comments addressed PTAL.

pedro93 · 2017-06-10T16:42:43Z

The failed test looks weird, is the CI environment running performance tests to validate the build?

ebidel · 2017-06-10T18:02:25Z

chrome-launcher/README.md

+Launch chrome with ease from node.
+
+```
+npm install chrome-launcher


yarn add chrome-launcher # npm install chrome-launcher

ebidel · 2017-06-10T18:03:17Z

chrome-launcher/README.md

+chromeLauncher.launch({
+  // optional staring url string
+  startingUrl: string;
+  // optional array of (string) flags to pass to chrome


mention these need -- or link to http://peter.sh/experiments/chromium-command-line-switches/

ebidel · 2017-06-10T18:03:50Z

chrome-launcher/README.md

+  startingUrl: string;
+  // optional array of (string) flags to pass to chrome
+  chromeFlags: Array<string>;
+  // optional explicit remote debugging port number to use.


By default, a free port is selected for you.

ebidel · 2017-06-10T18:04:15Z

chrome-launcher/README.md

+#### Launching chrome:
+
+```js
+const chromeLauncher = require('chrome-launcher')


ebidel · 2017-06-10T18:04:53Z

chrome-launcher/README.md

+#### Launching headless chrome:
+
+```js
+const chromeLauncher = require('chrome-launcher')


ebidel · 2017-06-10T18:05:17Z

chrome-launcher/README.md

+
+chromeLauncher.launch({
+  startingUrl: 'https://google.com',
+  chromeFlags: ['--headless']


chromeFlags: ['--headless', '--disable-gpu'] for now.

ebidel · 2017-06-10T18:05:46Z

chrome-launcher/package.json

-    "@types/node": "6.0.66"
-  }
+    "@types/node": "6.0.66",
+    "lighthouse": "^2.0.0"


ebidel · 2017-06-10T18:07:58Z

Also fixes #2469.

@samccone Can you also mention the publishing workflow in https://github.com/GoogleChrome/lighthouse/blob/master/CONTRIBUTING.md?

samccone · 2017-06-10T19:46:34Z

@ebidel noted how to release it in the doc.

ebidel · 2017-06-10T20:26:07Z

CONTRIBUTING.md

+
+```sh
+cd chrome-launcher
+echo "edit package.json bumping the version"


npm version major|minor|patch?

ebidel

LGTM. Just the last suggestion.

denar90 · 2017-06-10T20:48:02Z

@ebidel will be nice to update https://developers.google.com/web/updates/2017/04/headless-chrome after publishing this module ;)

ebidel · 2017-06-10T20:51:21Z

It was updated a few days ago to use the new chrome launcher. When the module is ready, we can switch over to showing install commands for that.

addressed feedback

patrickhulce · 2017-06-12T19:10:10Z

CONTRIBUTING.md

+```sh
+cd chrome-launcher
+echo "edit package.json bumping the version (major|minor|patch)"
+echo "commit the change as a version bump"


how about suggesting npm version (major|minor|patch)?

already updated :)

patrickhulce · 2017-06-12T19:11:26Z

chrome-launcher/.npmignore

+
+# dev files
+.appveyor.yml
+.DS_Store


patrickhulce · 2017-06-12T19:15:09Z

chrome-launcher/.npmignore

+node_modules/
+
+# include all compiled JS
+!*.js


the ignore files won't be merged though, is the parent folder ignore creeping in?

paulirish · 2017-06-13T16:28:29Z

CONTRIBUTING.md

+echo "build the launcher source code"
+yarn build
+echo "run npm version [<newversion> | major | minor | patch | premajor | preminor | prepatch | prerelease | from-git]"
+echo "commit the change as a version bump"


i prefer versioning with yarn because it's interactive, but w/e. also in both cases the commit (and tag) is made automatically.

yarn version echo "version bump will be committed"

…se#2358) * prep to publish to npm * Use logger from lighthouse. * Add launcher README * Add notes for releasing chrome launcher * Launcher readme tweaks * docs (contributing): launcher rls tweak

samccone requested a review from paulirish May 25, 2017 03:14

denar90 reviewed May 25, 2017

View reviewed changes

patrickhulce reviewed May 25, 2017

View reviewed changes

paulirish previously requested changes May 26, 2017

View reviewed changes

samccone force-pushed the prep-export branch 5 times, most recently from 5a53ddb to d53b8e8 Compare June 10, 2017 15:44

ebidel reviewed Jun 10, 2017

View reviewed changes

samccone force-pushed the prep-export branch from d53b8e8 to 960ab63 Compare June 10, 2017 19:46

ebidel reviewed Jun 10, 2017

View reviewed changes

ebidel suggested changes Jun 10, 2017

View reviewed changes

ebidel approved these changes Jun 10, 2017

View reviewed changes

samccone force-pushed the prep-export branch from 960ab63 to ab59f12 Compare June 12, 2017 00:09

samccone force-pushed the prep-export branch from ab59f12 to e7373d5 Compare June 12, 2017 19:12

patrickhulce reviewed Jun 12, 2017

View reviewed changes

paulirish and others added 3 commits June 12, 2017 12:21

prep to publish to npm

eb4af86

Use logger from lighthouse.

3fb556e

Add launcher README

ed165c2

samccone force-pushed the prep-export branch from e7373d5 to d361bf4 Compare June 12, 2017 19:21

Add notes for releasing chrome launcher

0d9738a

samccone force-pushed the prep-export branch from d361bf4 to 0d9738a Compare June 12, 2017 21:21

Launcher readme tweaks

2aa9734

paulirish reviewed Jun 13, 2017

View reviewed changes

docs (contributing): launcher rls tweak

cc8589c

paulirish approved these changes Jun 13, 2017

View reviewed changes

paulirish merged commit 224c6a0 into master Jun 13, 2017

paulirish deleted the prep-export branch June 13, 2017 17:23

westy92 mentioned this pull request Jun 17, 2017

Release logger as its own module #2526

Closed

Prep export of launcher as a standalone module #2358

Prep export of launcher as a standalone module #2358

Conversation

samccone commented May 25, 2017 • edited Loading

googlebot commented May 25, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulirish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

denar90 commented May 28, 2017

samccone commented Jun 10, 2017

pedro93 commented Jun 10, 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

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 Jun 10, 2017

samccone commented Jun 10, 2017

ebidel Jun 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebidel left a comment

Choose a reason for hiding this comment

denar90 commented Jun 10, 2017

ebidel commented Jun 10, 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

samccone commented May 25, 2017 •

edited

Loading

ebidel Jun 10, 2017 •

edited

Loading