-
-
Notifications
You must be signed in to change notification settings - Fork 278
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 support for Python 3.8 #2443
Conversation
for more information, see https://pre-commit.ci
pre-commit.ci autofix |
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.
Releasing the review before completion because I don't want to comment n time the same thing, if I'm misguided in saying that every condition that has PY38 and
something else can be removed entirely.
# The ast parser of python < 3.8 sets col_offset of multi-line strings to -1 | ||
# as it is unable to determine the value correctly. We reset this to None. | ||
if doc_ast_node.col_offset == -1: | ||
doc_ast_node.col_offset = None |
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.
Ho, this one is going to be a nice cleanup in pylint's testutil / functional !
Yeah, I just caught that myself. Sorry! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2443 +/- ##
==========================================
- Coverage 92.79% 92.75% -0.04%
==========================================
Files 94 94
Lines 11118 11045 -73
==========================================
- Hits 10317 10245 -72
+ Misses 801 800 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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'd probably have done that in multiple steps / PRs, like
- Update CI + do code changes to remove 3.8 special cases
- Update tests
- Update typing + ruff and pylint to 3.9.0
Might be easier to follow when reviewing it later, but this works the same.
--
In general, I do support removing support for 3.8. We could even do it fairly soon IMO. Maybe after the PEP 695 fix is merged and one last bugfix release created?
For pylint, we should just make sure to bump the minor version and update requires-python
when we pull in the new astroid release. The code changes to remove 3.8 can follow later as well.
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.
LGTM! Thanks @jacobtylerwalls 👍🏻
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.
@@ -74,20 +73,6 @@ | |||
except AttributeError: | |||
pass | |||
|
|||
if IS_PYPY and sys.version_info < (3, 8): |
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 catch, look like we forgot to remove when dropping 3.7
3.8 is EOL in late October 2024, and 3.13 final is scheduled for early October 2024. Aiming for new minor releases of astroid and pylint by then to drop support for 3.8 and add support for 3.13.