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

Add official Python 3.7 support #4717

Merged
merged 8 commits into from
Jul 24, 2018
Merged

Add official Python 3.7 support #4717

merged 8 commits into from
Jul 24, 2018

Conversation

ericwb
Copy link
Contributor

@ericwb ericwb commented Jun 28, 2018

Python 3.7 was just released [1]. This is a small change to
enable support in requests.

[1] https://docs.python.org/3.7/whatsnew/3.7.html

Signed-off-by: Eric Brown [email protected]

Python 3.7 was just released [1]. This is a small change to
enable support in requests.

Signed-off-by: Eric Brown <[email protected]>
@sigmavirus24
Copy link
Contributor

Travis doesnt' appear to support 3.7 yet: https://travis-ci.org/requests/requests/jobs/397660384

@ericwb
Copy link
Contributor Author

ericwb commented Jun 28, 2018

Looks like it's being worked: travis-ci/travis-ci#9815

@sigmavirus24
Copy link
Contributor

I see nothing indicating that it's in progress on Travis' part other than an issue that non-Travis-CI employees are commenting on.

@nateprewitt
Copy link
Member

Hey @ericwb, we’ll likely want this enabled for appveyor as well before we claim support for 3.7.

I haven’t seen an update from them but that’d be something to look into before we merge this.

@codecov-io
Copy link

codecov-io commented Jul 2, 2018

Codecov Report

Merging #4717 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4717   +/-   ##
=======================================
  Coverage   66.79%   66.79%           
=======================================
  Files          15       15           
  Lines        1563     1563           
=======================================
  Hits         1044     1044           
  Misses        519      519

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 4d51e6d...2aa29f8. Read the comment docs.

@jdufresne
Copy link
Contributor

According to appveyor/ci#2475, Appveyor now supports Python 3.7.

@nateprewitt
Copy link
Member

@jdufresne great to hear! Once we get a working build with that, I’m happy to merge.

@nateprewitt
Copy link
Member

Hey @ericwb, wanted to check in on this again if you get a minute. Otherwise, I'm going to look at cherry picking these onto master with the appveyor integration later this week. Thanks!

sigmavirus24
sigmavirus24 previously approved these changes Jul 20, 2018
@sigmavirus24
Copy link
Contributor

@ericwb would you be willing to rebase this?

@nateprewitt
Copy link
Member

And squash the commits too if you wouldn’t mind. Thanks!

.travis.yml Outdated
@@ -22,8 +13,35 @@ jobs:
- stage: test
script:
- |
if [[ "$TRAVIS_PYTHON_VERSION" != "2.6" ]] ; then make test-readme; fi
make test-readme
Copy link
Contributor

Choose a reason for hiding this comment

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

This could drop the | and fit on one line

.travis.yml Outdated
- stage: test
script:
- |
make test-readme
Copy link
Contributor

Choose a reason for hiding this comment

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

Same.

.travis.yml Outdated
- stage: test
script:
- |
make test-readme
Copy link
Contributor

Choose a reason for hiding this comment

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

Same.

.travis.yml Outdated
- stage: test
script:
- |
make test-readme
Copy link
Contributor

Choose a reason for hiding this comment

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

Same.

.travis.yml Outdated
- stage: test
script:
- |
make test-readme
Copy link
Contributor

Choose a reason for hiding this comment

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

Same.

.travis.yml Outdated
- make ci
python: '3.7'
dist: xenial
sudo: true
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need sudo here and does this conflict with the sudo: False declaration above?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, it does not. This is the only way to use xenial on Travis is to use sudo on this job alone. And xenial is the only target that has true Python 3.7. So it's not ideal (because enabling sudo is slow) but it's the best we have right now

@@ -1,14 +1,5 @@
sudo: false
language: python
python:
Copy link
Member

Choose a reason for hiding this comment

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

Could we get a quick explanation of the benefits/trade offs of moving from the matrix approach to this new setup?

Copy link
Contributor

Choose a reason for hiding this comment

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

You mean moving from the old configuration to the more explicit matrix definition? Other than greater flexibility?

Copy link
Member

@nateprewitt nateprewitt left a comment

Choose a reason for hiding this comment

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

Alright, I think this looks good to go. Thanks for seeing this all the way through, @ericwb!

@nateprewitt nateprewitt merged commit 6686ac1 into psf:master Jul 24, 2018
@mrkz
Copy link

mrkz commented Aug 7, 2018

@nateprewitt any release coming soon with python3.7 support?
Thanks in advance!

@nateprewitt
Copy link
Member

nateprewitt commented Aug 7, 2018

@mrkz, 2.19.1 should already support 3.7. This was just adding documentation stating that.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants