-
Notifications
You must be signed in to change notification settings - Fork 12
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
Python 3 changes plus automated testing #5
Conversation
@@ -10,4 +10,3 @@ junit-xml = "*" | |||
[dev-packages] | |||
|
|||
[requires] | |||
python_version = "2.7" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this not be "2.7,3.7", to reflect what's being used in the Travis config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that leaving it out is in alignment with the docs... https://pipenv-fork.readthedocs.io/en/latest/basics.html#specifying-versions-of-python
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, sure :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm.
As long as this continues to work on py2 and py3, it LGTM. Who has credentials to land it? @cclauss once landed, do you have pypi publish credentials now? |
Only @jbergstroem can land it and only @jbergstroem has the PyPI credentials. |
Alternative plan if this isn't landed soon, as this is a blocker? |
See: nodejs/build#1921 for all the places in ansible scripts where it is installed. I kinda hate vendoring in. |
We could also publish the fork on PyPi under I realise neither are great solutions, but we should have a plan if this is a blocker for moving to Python 3. |
@jbergstroem In the interests of minimizing maintenance burden for you, if you are willing, you could just add either @cclauss , or me (@sam-github), or any other Node.js TSC member to admin on this repo, I think that would allow us to do the maintenance. But if you have time to add cclauss to pypi and transfer it yourself, that works great, too! I'm not a fan of ansible, and would be happy to vendor it, but I'm not keen to grout out all the references to it in our ansible scripts. The low-maintenance fix is definitely to transfer it, if Johan is willing. |
@cclauss I think you can land this, can you try? |
PR-URL: nodejs#5 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Matt (IPv4) Cowley <me@mattcowley.co.uk>
PR-URL: #5 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Matt (IPv4) Cowley <me@mattcowley.co.uk>
Floating patch for Python 3 support. PR-URL: #5 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Matt (IPv4) Cowley <me@mattcowley.co.uk>
Floating patch for Python 3 support. PR-URL: #5 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Matt (IPv4) Cowley <me@mattcowley.co.uk>
This seems to be blocking Python 3 progress at nodejs/node#29451 (comment)
Test results: https://travis-ci.com/cclauss/tap2junit
@jbergstroem Your review please. Will require a new release to PyPI #8
@sam-github @refack @bnoordhuis @Trott Your reviews too please.