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.

Sign up for GitHub

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

Add support for Python 3.13 #3460

Merged
merged 2 commits into from
Dec 23, 2024
Merged

Add support for Python 3.13 #3460

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5812a56
Add support for Python 3.13
hugovk Mar 14, 2024
55c9247
Retain CI branch restriction
hugovk Dec 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/test-suite.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ name: Test Suite

on:
push:
branches: ["master"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did this change? I don't think it should?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was so I could run the CI on my feature branch and be confident it passes before opening a PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 could you revert it? I think this should be covered by the pull_request target.

Copy link
Contributor

@zanieb zanieb Dec 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Separately, we could add a workflow_dispatch trigger so it can be invoked on fork branches manually?

Copy link
Contributor Author

@hugovk hugovk Dec 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, happy to revert it. It just means next time I'll either make my changes directly on my main (not recommended) or not bother contributing (also not recommended :)

pull_request alone doesn't cover it: if I push changes to my fork, the CI won't run.

workflow_dispatch is good to have anyway, but I don't want to have to manually and tediously click buttons to trigger builds each and every time.

Copy link
Contributor

@zanieb zanieb Dec 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Usually I just open a pull request in my fork for CI if I need to test it before opening a real pull request.

I'm not sure exactly what the conditions cause this, but having push without a filter and pull_request can run jobs twice on each pull request commit e.g. astral-sh/python-build-standalone@accb28c

Screenshot 2024-12-23 at 1 16 17 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, creating a PR in your own fork is possible, it's still an extra impediment to contribution. This project's contribution guidelines shown how to test locally, which is good.

But we get all these free CI minutes, I'm a big fan of CIs, and my opinion is we should encourage people to test on CI and make it easy for them. No "works on my machine". Let me easily test all the Python version, all the operating systems, all the build steps in a clean CI environment. Let's have good "DX".

And it's possible to add config to prevent those double runs on upstream, I can look it up if you like.

Anyway, I've reverted this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

And it's possible to add config to prevent those double runs on upstream, I can look it up if you like.

I'm open to a proposal. My main reactions are that it doesn't belong in this pull request / limiting to the main branch is the standard pattern in all the repositories I work in. I agree making it easy for developers to test in CI is nice though :)

pull_request:
branches: ["master", "version-*"]

Expand All @@ -14,7 +13,7 @@ jobs:

strategy:
matrix:
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"]
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"]

steps:
- uses: "actions/checkout@v4"
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ classifiers = [
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: 3.12",
"Programming Language :: Python :: 3.13",
"Topic :: Internet :: WWW/HTTP",
]
dependencies = [
Expand Down
Loading