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

Make twine Python 3 only #437

Merged
merged 5 commits into from
Sep 22, 2019
Merged

Make twine Python 3 only #437

merged 5 commits into from
Sep 22, 2019

Conversation

jdufresne
Copy link
Contributor

I would humbly like to suggest the twine project move to be Python 3 only. As twine is a command-line tool, it is in a good position to move to Python 3 only. Other command-line-only tools have begun moving to Python 3. For example: black, mypy, pylint, pypinfo, and Sphinx. The twine command will continue to support uploading Python 2 projects to PyPI. So this should be mostly transparent to users.

Allows for code cleanups. By removing these workarounds, the project will be easier to maintain and reduce ongoing testing resources.

Python 2 is scheduled to be EOL on January 1, 2020.

https://www.python.org/dev/peps/pep-0373/

For an overview of projects that have dropped Python 2 support entirely, see:

https://hugovk.github.io/drop-python/2.7/

@codecov
Copy link

codecov bot commented Dec 8, 2018

Codecov Report

Merging #437 into master will decrease coverage by 0.61%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #437      +/-   ##
=========================================
- Coverage   84.91%   84.3%   -0.62%     
=========================================
  Files          14      14              
  Lines         809     771      -38     
  Branches      121     118       -3     
=========================================
- Hits          687     650      -37     
  Misses         84      84              
+ Partials       38      37       -1
Impacted Files Coverage Δ
twine/commands/__init__.py 100% <ø> (ø) ⬆️
twine/cli.py 100% <ø> (ø) ⬆️
twine/wininst.py 27.77% <ø> (-3.81%) ⬇️
twine/__init__.py 100% <ø> (ø) ⬆️
twine/package.py 88% <100%> (-2.16%) ⬇️
twine/settings.py 93.22% <100%> (-0.12%) ⬇️
twine/commands/upload.py 92.45% <100%> (-0.28%) ⬇️
twine/wheel.py 82.22% <100%> (-2.1%) ⬇️
twine/commands/register.py 56.52% <100%> (-1.82%) ⬇️
twine/commands/check.py 98.61% <100%> (-0.04%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3c1523...44e9eb6. Read the comment docs.

Copy link
Member

@jaraco jaraco left a comment

Choose a reason for hiding this comment

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

In general this looks good. It does conflate some stylistic changes with meaningful changes, and on the whole the changes look preferable. The changes could have been more conservative and still achieve the goal, but I'm largely in favor of the request. I've made some select comments for your consideration, but none are hard blockers.

setup.cfg Show resolved Hide resolved
tests/test_wheel.py Show resolved Hide resolved
twine/__main__.py Outdated Show resolved Hide resolved
twine/cli.py Outdated Show resolved Hide resolved
twine/cli.py Outdated Show resolved Hide resolved
twine/cli.py Outdated Show resolved Hide resolved
twine/commands/check.py Outdated Show resolved Hide resolved
twine/wheel.py Show resolved Hide resolved
@jaraco
Copy link
Member

jaraco commented Jan 8, 2019

Aha. And now I see that in #436, you've done just as I'd have - added the Python 2 compatible changes first.

@jdufresne
Copy link
Contributor Author

Thanks very much for the review @jaraco! I have rebased on the latest master so this PR no longer includes Python 2 style changes and extracted the io.StringIO change as suggested. I have responded to the comments that I believe should remains as-is. Please let me know if you have additional feedback or suggestions. 🙂

@sigmavirus24
Copy link
Member

I want to hold off on this until we can cut a 2.0 and I want to cut a different release today

@jdufresne
Copy link
Contributor Author

I have rebased to resolve merge conflicts. I also adjusted the tox environment "release" with basepython = python3 to help further mitigate the concern with mistakenly releasing a Python 2 wheel.

I want to hold off on this until we can cut a 2.0 and I want to cut a different release today

Sounds good to me.

@jaraco
Copy link
Member

jaraco commented Jun 8, 2019

I'm ready to accept this. @theacodes do you have any objections?

@sigmavirus24
Copy link
Member

Isn't py3.4 EOL? Should we raise the minimum supported version to 3.5 or 3.6?

@jdufresne
Copy link
Contributor Author

Yeah, I agree. I've removed Python 3.4 support in the latest revision.

@bhrutledge
Copy link
Contributor

I'm working on type annotations for #231. If twine required 3.6+, we could use the new syntax for variable annotations, instead of type comments. I don't know what other considerations there might be for dropping 3.5.

@theacodes
Copy link
Member

theacodes commented Jun 12, 2019 via email

The twine project is moving to Python 3 only.

As twine is a command-line tool, it is in a good position to move to
Python 3 only. Other command-line-only tools have begun moving to Python
3. For example: black, mypy, pylint, pypinfo, and Sphinx. The twine
command will continue to support uploading Python 2 projects to PyPI. So
this should be mostly transparent to users.

Allows for code cleanups. By removing these workarounds, the project
will be easier to maintain and reduce ongoing testing resources.

Python 2 is scheduled to be EOL on January 1, 2020.

https://www.python.org/dev/peps/pep-0373/

For an overview of projects that have dropped Python 2 support entirely,
see:

https://hugovk.github.io/drop-python/2.7/
@jdufresne
Copy link
Contributor Author

I have added a commit to remove Python 3.5 support.

Allows dropping dependency on pyblake2, as it is now available in
hashlib.

Allows using f-strings instead of simple string formatting.

Allows for future use of type hinting.

PyPy3 is Python 3.5, so that support has been removed as well.
@bhrutledge
Copy link
Contributor

@jaraco Any more thoughts about merging this? Among other things, it'd be nice to use as a baseline for adding type annotations.

@bhrutledge bhrutledge mentioned this pull request Jul 24, 2019
@bhrutledge
Copy link
Contributor

@pypa/twine-maintainers How do y'all feel about merging this? It makes Twine require Python 3.6+, and drops support for PyPy.

@di
Copy link
Member

di commented Aug 9, 2019

@sigmavirus24 said above:

I want to hold off on this until we can cut a 2.0 and I want to cut a different release today

I agree that this belongs in a 2.x release, and I think we should continue merging and releasing 1.x until the EOL, so at the earliest I'd support merging & releasing this on Jan 1, 2020.

@jaraco
Copy link
Member

jaraco commented Aug 9, 2019

My preference would be to merge this and cut a release, but maintain a 1.x branch for bug fixes or other contributions deemed worthy of the effort.

@sigmavirus24
Copy link
Member

Right. I think at that time there was a "We'd like to rush something out" release and now is a perfectly fine time to release 2.x. We can branch a 1.x branch as @jaraco says and backport to that until Jan 1, 2020 as necessary.

@bhrutledge bhrutledge mentioned this pull request Sep 7, 2019
@jaraco
Copy link
Member

jaraco commented Sep 7, 2019

I was actually hoping to roll this out today, but it's blocked on #468.

@jaraco
Copy link
Member

jaraco commented Sep 22, 2019

I've created a maint/1.x branch for maintaining changes for 1.x releases as needed.

@jaraco jaraco merged commit fdffae1 into pypa:master Sep 22, 2019
setup.py Show resolved Hide resolved
@di di mentioned this pull request Sep 23, 2019
@jdufresne jdufresne deleted the py3 branch January 19, 2020 00:06
dochang added a commit to dochang/dotfiles that referenced this pull request Apr 3, 2020
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.

6 participants