-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
Add support for Python 3.8-3.10, drop EOL 2.7-3.5 #42
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
84c2dfb
Add support for Python 3.8-3.10
hugovk 44c111e
Remove redundant Travis CI config, GHA is used for CI
hugovk 85d84d7
Update build badge for GHA
hugovk 3a4a029
Drop the dot https://twitter.com/pytestdotorg/status/753767547866972160
hugovk b2117c1
Bump attrs, pluggy and pytest pins to support Python 3.10
hugovk 763ce7f
Drop support or EOL Python 2.7-3.5
hugovk 2301ca9
Upgrade Python syntax with pyupgrade --py36-plus
hugovk 051e79d
Drop support for EOL Python 2.7
hugovk c54ad60
Remove redundant six
hugovk e12bd75
Tidy Python 3+ imports
hugovk c37ac89
http to https
hugovk 98c852f
Update docs_requirements.txt
tomchristie File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,3 @@ | ||
from __future__ import absolute_import | ||
import sys | ||
|
||
# This is the canonical package information. | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
On the top of this file there's a "on: [push]".
Can you replace by this: https://github.com/encode/starlette/blob/bc61505faef15b673bf4bf65db4927daa354d6b8/.github/workflows/test-suite.yml#L4 ?
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'm personally strongly against adding this, it would have prevented me testing the CI on my fork and is an extra impediment to contributing.
https://github.com/hugovk/python-multipart/actions
Unless I edited the yml in my fork, and then remembered to revert it before creating a PR.
Or I can submit a PR on my branch first to check it passes -> extra impediment.
Or I can develop on my
master
-> not a best practice, we should use feature branches, and I only have a singlemaster
-> extra impediment.The fact it's tested on upstream and on the fork isn't such a big deal: the testing is split between two different accounts and therefore doesn't use up any extra quota/time from any single account.
CIs allow us to test on a much larger matrix of Python versions and operating systems, much more than is possible locally and sometimes faster too. I sometimes avoid contributing to projects with no/broken CI.
I believe the use of CIs are of huge benefit for software quality and development, especially when so easy nowadays; Travis CI really revolutionised this, remember how hard testing was before. We should rather encourage people to test on their own forks before creating PRs.
Fail fast: shorten the time to finding a failure, enable quicker loops of incremental development.
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.
You're not against this. You're against GitHub blocking first time contributors for running their pipeline.
There's a setting to allow old GitHub users (3 months old ?) to not be blocked.
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 don't mind that bit too much :)
Consider this workflow:
I come along and fork the repo and enable the CI on my fork.
I create a feature branch
add-3.10
.I push my feature branch to my fork and check the CI.
I want the CI to test my changes on all supported Python versions (and possibly on several operating systems).
If it fails, I'll iterate: fix, push, repeat.
(This actually happened, see the first few failures as I iterated: https://github.com/hugovk/python-multipart/actions)
Once it passes, I can create a PR here, knowing that when the CI is enabled for the PR (we're 10 months and counting here), I'm confident it will pass and there's no surprises because I didn't test Python 3.7 or so on.
However, if we have these branch restrictions:
The CI won't build for me.
My options:
master
branch - not recommended, you can only work on one thingAll this applies even if I'm a second-time contributor.