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

Drop Python 2 support #1179

Merged
merged 35 commits into from
Feb 22, 2022
Merged

Drop Python 2 support #1179

merged 35 commits into from
Feb 22, 2022

Conversation

itamarst
Copy link
Collaborator

@itamarst itamarst commented Feb 14, 2022

Fixes https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3873
Fixes https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3327

  • CircleCI has no Python 2
  • GitHub Actions has no Python 2
  • Modern Fedora
  • Oracle Linux as replacement for CentOS, since the latter is dead
  • Switch image building back to schedule
  • Everything passes
  • Update setup.py (metadata plus minimum version)
  • Update README
  • REVIEWER: Anything else that needs updating?
  • REVIEWER: Remove old checks from requirements for branch (needs to be done by @meejah or @exarkun since I lack permissions)

https://www.youtube.com/watch?v=aiwA0JrGfjA

Temporarily switch image building to always happen.
@itamarst itamarst marked this pull request as draft February 14, 2022 15:36
@itamarst itamarst changed the title Try to switch to modern Python 3 world. WIP: drop Python 2 builders Feb 14, 2022
@coveralls
Copy link
Collaborator

coveralls commented Feb 14, 2022

Coverage Status

Coverage decreased (-1.1%) to 94.546% when pulling 5647b4a on 3873-welcome-to-the-world-of-tomorrow into 296bc3e on master.

@itamarst itamarst changed the title WIP: drop Python 2 builders Drop Python 2 support Feb 15, 2022
@itamarst itamarst marked this pull request as ready for review February 15, 2022 15:59
@itamarst itamarst requested a review from a team February 15, 2022 15:59
Copy link
Member

@exarkun exarkun left a comment

Choose a reason for hiding this comment

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

Thanks. I left some comments inline.

.circleci/Dockerfile.oraclelinux Show resolved Hide resolved
.circleci/config.yml Show resolved Hide resolved
.circleci/config.yml Show resolved Hide resolved
newsfragments/3873.incompat Outdated Show resolved Hide resolved
@@ -201,8 +188,7 @@ def read_version_py(infname):
"Natural Language :: English",
"Programming Language :: C",
"Programming Language :: Python",
"Programming Language :: Python :: 2",
"Programming Language :: Python :: 2.7",
"Programming Language :: Python :: 3",
Copy link
Member

Choose a reason for hiding this comment

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

Are there any best practices for when to include minor version numbers too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🤷 not sure anyone actually looks at these anymore, the python_requires is the important bit.

setup.py Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
pyinstaller.spec Show resolved Hide resolved
@itamarst
Copy link
Collaborator Author

Not sure what's up on macOS. Will have to spin up tahoe dev environment on my macOS VM I guess :(

@exarkun
Copy link
Member

exarkun commented Feb 22, 2022

I am now looking at Settings > Branches > Branch Protection Rules > master > Edit > Require status checks to pass before merging and I am clicking the x next to:

  • ci/circleci: centos-8 (centos is gone, hello oraclelinux)
  • ci/circleci: debian-9 (debian 9 is gone, hello debian 11)
  • ci/circleci: fedora-28 (fedora 28 is gone, hello fedora 35)
  • ci/circleci: fedora-29 (fedora 29 is gone, hello fedora 35)
  • ci/circleci: lint (lint is gone, hello codechecks)
  • ci/circleci: python37 (it's all python3 now)
  • ci/circleci: ubuntu-16-04 (ubuntu 16.04 is gone, hello 20.04)
  • coverage (macos-10.15, 2.7) (python2.7 is gone, hello coverage (macos-latest, 3.9))
  • coverage (windows-latest, 2.7) (python2.7 is gone, hello coverage (windows-latest, 3.7, 3.8. 3.9))
  • integration (macos-10.15, 2.7) (python2.7 is gone, hello integration (macos-latest, 3.9))
  • integration (windows-latest, 2.7) (python2.7 is gone, hello integration (windows-latest, 3.7, 3.9))
  • packaging (macos-10.15, 2.7) (python2.7 is gone, hello packaging (macos-10.15, 3.9))
  • packaging (ubuntu-latest, 2.7) (python2.7 is gone, hello packaging (ubuntu-latest, 3.9))
  • packaging (windows-latest, 2.7) (python2.7 is gone, hello packaging (windows-latest, 3.9))

These replacements mostly seem pretty good. One thing that stands out looking at this list is that we're not very consistent in whether we pick macos-latest or macos-10.15. We used to pick macos-10.15 everywhere (even though we pick windows-latest and ubuntu-latest). Now we use both macos-10.15 and macos-latest. Maybe that's as it must be to get the necessary versions of Python? But from https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners I see that macos-11 should currently be the same as macos-latest - maybe we should prefer macos-11 to avoid getting shifted to a newer version whenever github feels like it? Of course this same reasoning could apply to the Windows and Ubuntu images... However, I can't say I'm familiar enough with GitHub's image update process to know if this is a real issue or not. Probably makes sense to leave this as-is until we know a reason to do it differently, unless you already do...

And now I have clicked the "Save changes" button.

Copy link
Member

@exarkun exarkun left a comment

Choose a reason for hiding this comment

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

Thanks!

@itamarst itamarst merged commit 2444c55 into master Feb 22, 2022
@itamarst itamarst deleted the 3873-welcome-to-the-world-of-tomorrow branch February 22, 2022 18:06
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.

3 participants