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 3.6 #307

Merged
merged 3 commits into from Dec 27, 2021
Merged

Drop Python 3.6 #307

merged 3 commits into from Dec 27, 2021

Conversation

Andrew-Chen-Wang
Copy link
Contributor

Python 3.6 EOL is soon (apparently it's today!). Dropping some compatibility. Hoping to add a PR for Local soon.

@Andrew-Chen-Wang Andrew-Chen-Wang marked this pull request as draft December 23, 2021 05:54
@Andrew-Chen-Wang Andrew-Chen-Wang marked this pull request as ready for review December 23, 2021 06:23
@Kludex
Copy link
Contributor

Kludex commented Dec 23, 2021

We should remove the vendored pep as well

@andrewgodwin
Copy link
Member

Lovely, this is a pretty significant cleanup. You love to see it.

@andrewgodwin andrewgodwin merged commit 02fecb6 into django:main Dec 27, 2021
@Andrew-Chen-Wang
Copy link
Contributor Author

@Kludex do you mean pep562? What did I miss?

@Kludex
Copy link
Contributor

Kludex commented Dec 27, 2021

Ah, you did remove it! Sorry. For some reason I didn't see the full diff before.

@Andrew-Chen-Wang
Copy link
Contributor Author

np. It's prob because deleted files don't show a full diff, only a sliver of gray with the file name on GitHub.

@andrewgodwin
Copy link
Member

Yeah, the diff and removal of the vendored backport all looked good to me!

@akx
Copy link
Contributor

akx commented Jan 4, 2022

There's still this bit in asgiref/local.py:

asgiref/asgiref/local.py

Lines 29 to 30 in 02fecb6

This doesn't use contextvars as it needs to support 3.6. Once it can support
3.7 only, we can then reimplement the storage more nicely.

A future PR should probably make that module use contextvars then :)

@Andrew-Chen-Wang
Copy link
Contributor Author

Yes, a separate PR is needed. I lost a lot of time due to start of school, so I can't make that local PR. Sorry.

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.

None yet

4 participants