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

Windows: Look into LIGHTHOUSE_CHROMIUM_PATH variable too. #1572

Merged
merged 1 commit into from
Feb 1, 2017
Merged

Windows: Look into LIGHTHOUSE_CHROMIUM_PATH variable too. #1572

merged 1 commit into from
Feb 1, 2017

Conversation

XhmikosR
Copy link
Contributor

Fixes #1472.

@paulirish
Copy link
Member

sg. shall we add it for Mac too just so it's straightforward to reason about?

if this is set, then we should definitely use it rather than any sniffed location.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Jan 30, 2017

I guess it makes sense to take into consideration the variable. Although it should be documented somewhere.

I need this for #1280 and noticed that only on Linux the env var was taken into consideration.

@brendankenny
Copy link
Member

+1 for mac support and having it override any other choice (unless you use --select-chrome in addition to setting the variable).

win32 will need the priorities treatment that the other platforms get, and mac will need to set process.env.LIGHTHOUSE_CHROMIUM_PATH as highest priority in the same way linux does

@XhmikosR
Copy link
Contributor Author

@brendankenny: what does that mean? That this won't be merged currently?

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.

actually, I guess this does handle with and without --select-chrome since without a sort step the value in LIGHTHOUSE_CHROMIUM_PATH will win, as you would expect.

We can handle macs and documenting the variable in the future.

LGTM

@brendankenny brendankenny merged commit 5bbab8d into GoogleChrome:master Feb 1, 2017
@XhmikosR XhmikosR deleted the LIGHTHOUSE_CHROMIUM_PATH branch February 1, 2017 08:30
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.

None yet

3 participants