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

lib: use print() for python version detection #1534

Merged
merged 1 commit into from
Oct 20, 2018

Conversation

greenaddress
Copy link
Contributor

@greenaddress greenaddress commented Aug 27, 2018

This fixes an issue introduced by commit da5db40 specifically this line da5db40#diff-7ad6309831a48a234780b58baca65710R440

Namely, the bit that checks for python versions was changed and went from supporting both python2 and python3 to supporting only python2.

I understand there are other open issues/PRs in regards to python3 but this change specifically is a regression that broke windows builders with python3 in the path as python.

@refack
Copy link
Contributor

refack commented Aug 28, 2018

Hello @greenaddress, and thank you for the contribution.
Code LGTM, but could format the commit message according to our convention. Something like:

lib: use print() for python version detection

@greenaddress
Copy link
Contributor Author

@refack done, thank you for review!

@refack refack changed the title use more generic print with parathesis as supported by more python ve… lib: use print() for python version detection Aug 30, 2018
@refack
Copy link
Contributor

refack commented Aug 30, 2018

@greenaddress thank you for following up. We'll land this soon, then we need to release a new version and get that landed in npm so it might take some time for this to get to the public.

@jkoorts
Copy link

jkoorts commented Jan 16, 2019

Still happening 5 months later..

@Fylipp
Copy link

Fylipp commented Feb 19, 2019

Since this seems to be a very common error (countless forum posts and issues / who has Python 2 installed nowadays?) this should have been integrated into a package release by now. The last NPM release of node-gyp was 6 months ago. A small release containing this change would increase user friendliness considerably.

@mittalyashu
Copy link

I am still facing this issue, when this bug fix will be released.

@mittalyashu
Copy link

image

The last time an update was release 7 months ago, can you release the a new update with all the bug fixes

/cc @refack @hashseed @richardlau

@richardlau
Copy link
Member

I don't have permissions to publish this to npm.

@mittalyashu
Copy link

😮 Oh! Okay

richardlau pushed a commit that referenced this pull request Apr 12, 2019
PR-URL: #1534
Reviewed-By: Refael Ackermann <refack@gmail.com>
@refack refack mentioned this pull request Apr 12, 2019
3 tasks
@steveoh
Copy link

steveoh commented Jul 9, 2019

What versions did this go into? I'm on 10.16 and 6.10 and seeing this message?

@richardlau
Copy link
Member

What versions did this go into? I'm on 10.16 and 6.10 and seeing this message?

node-gyp 4: https://github.com/nodejs/node-gyp/blob/master/CHANGELOG.md#v400-2019-04-24

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.

9 participants