Skip to content

Try python launcher when stock python is python 3.#992

Merged
bnoordhuis merged 2 commits intonodejs:masterfrom
bnoordhuis:fix987
Oct 8, 2016
Merged

Try python launcher when stock python is python 3.#992
bnoordhuis merged 2 commits intonodejs:masterfrom
bnoordhuis:fix987

Conversation

@bnoordhuis
Copy link
Copy Markdown
Member

@bnoordhuis bnoordhuis commented Jul 13, 2016

@bnoordhuis
Copy link
Copy Markdown
Member Author

@rvagg
Copy link
Copy Markdown
Member

rvagg commented Jul 14, 2016

ack

@bnoordhuis
Copy link
Copy Markdown
Member Author

Rebased, new CI: https://ci.nodejs.org/view/All/job/nodegyp-test-commit/31/

@rvagg Was your ACK an implicit LGTM or an 'I am aware this pull request exists'?

@jbergstroem
Copy link
Copy Markdown
Member

LGTM

Break up findPython() into pieces that can be tested in isolation and
add tests that do so.

PR-URL: #992
Reviewed-By: Johan Bergström <[email protected]>
Consult the python launcher when the python on the path is python 3.
If a python 2 exists on the system, it will tell us.

Fixes: #987
PR-URL: #992
Reviewed-By: Johan Bergström <[email protected]>
@bnoordhuis bnoordhuis merged commit 37ae7be into nodejs:master Oct 8, 2016
@bnoordhuis bnoordhuis deleted the fix987 branch October 8, 2016 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants