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: demo flags in example of programmatic use #3841

Merged
merged 2 commits into from
Jan 11, 2018

Conversation

nhodges
Copy link
Contributor

@nhodges nhodges commented Nov 17, 2017

In the current documentation, the example given does not pass the flags argument to chromeLauncher.launch which caused some initial confusion when trying to leverage any of the chromeFlags. It is not intuitive that flags should also be passed into the launch method.

In the current documentation, the example given does not pass the flags argument to `chromeLauncher.launch` which caused some initial confusion when trying to leverage any of the chromeFlags. It is not intuitive that flags should also be passed into the launch method.
@vinamratasingal-zz
Copy link

Will let Paul make the final call here, but this is OK with me :)

docs/readme.md Outdated
@@ -12,7 +12,7 @@ const lighthouse = require('lighthouse');
const chromeLauncher = require('chrome-launcher');

function launchChromeAndRunLighthouse(url, flags = {}, config = null) {
return chromeLauncher.launch().then(chrome => {
return chromeLauncher.launch(flags).then(chrome => {
Copy link
Member

Choose a reason for hiding this comment

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

launch() takes an opts object

a prop of that object is chromeFlags .. so perhaps rename flags to launchOpts and add in a chromeFlags: ['--disable-popup-blocking'] into it for a clear example?

@paulirish
Copy link
Member

@nhodges thanks for this PR.. but there's an error in it. do you have the time to update it? thx! 🏖

@nhodges
Copy link
Contributor Author

nhodges commented Dec 18, 2017

@paulirish Hey, thanks for the feedback! Sorry I haven't gotten to it sooner -- was in the middle of a move. (Thanks for the nudge.) I'll gladly update my PR, probably tomorrow evening.

@paulirish
Copy link
Member

@nhodges nudge ;)

@GoogleChrome GoogleChrome deleted a comment from googlebot Jan 11, 2018
@paulirish paulirish merged commit fc5c9d7 into GoogleChrome:master Jan 11, 2018
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.

4 participants