-
Notifications
You must be signed in to change notification settings - Fork 10.6k
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 support #5514
Drop Python 3.6 support #5514
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5514 +/- ##
==========================================
- Coverage 88.72% 88.64% -0.08%
==========================================
Files 163 162 -1
Lines 10671 10667 -4
Branches 1819 1819
==========================================
- Hits 9468 9456 -12
- Misses 927 937 +10
+ Partials 276 274 -2
|
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.
Hi, thanks for the update. We still need the typing, pypy and pinned envs though, we need to update their versions instead of removing them completely.
@elacuesta Sure! I will change to use pypy version 3.7, what you think? |
Indeed that works, although I don't think we have a strong requirement for pypy to be any specific major version. That's not the case for the pinned one, ideally it should be the earliest version in the 3.7 series (that's supported by CI). |
Looks like pinned lxml is too old for 3.7? |
@wRAR Yeah, i will take a look later. |
@wRAR @elacuesta Had to bump some dependencies in order to make everything work with pinned 3.7. Don't know why codecov & checks (3.10, docs) are failing but is not related with the changes that i made. |
was anyone able to figure out why codecov/path check is failling? |
The docs env is failing in master too (517cbc8), I wouldn't worry about it for this PR. Regarding the coverage check, I suppose it could be possible that parts of the codebase are not reached now with the new requirement versions, but I'm not completely sure that's the case. We need to update |
Some lessons learned with this PR until now: Earliest supported release for pypy3.7 is v7.3.5, reason: Bumped Bumped |
Looking at https://app.codecov.io/gh/scrapy/scrapy/compare/5514/diff, that is an expected non-covered path (we decided long ago not to run a CI job just to verify that Scrapy fails nicely on an unsupported Python version). |
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.
Maybe we should also remove scrapy/utils/py36.py
here, it has been more than a year since it was first deprecated (Scrapy 2.5).
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.
Great job! I bet you did not anticipate it to be this involved 🙂
( |
You bet is right. 😅 |
@elacuesta Can you take a look? |
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.
Thanks!
Closes #5512