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

Require python 3.7.2+ #5921

Merged
merged 6 commits into from
Apr 11, 2022
Merged

Require python 3.7.2+ #5921

merged 6 commits into from
Apr 11, 2022

Conversation

DanielNoord
Copy link
Collaborator

@DanielNoord DanielNoord commented Mar 15, 2022

  • Add yourself to CONTRIBUTORS.txt if you are a new contributor.
  • Add a ChangeLog entry describing what your PR does.
  • If it's a new feature, or an important bug fix, add a What's New entry in
    doc/whatsnew/<current release.rst>.
  • Write a good description on what the PR does.

Type of Changes

Type
✨ New feature
📜 Docs

Description

Two additional follow up PRs would be necessary after this:

  1. Remove all tests options for min_pyver=3.7. That would be quite a large diff so I have excluded it from this PR.
  2. Update py-version in pylintrc. This causes a number of warnings about using from __future__ import annotations. So that warrants its own PR.
  3. We might want to run https://github.com/cdce8p/python-typing-update before doing step 2 just to see what can be automated

Just a draft to see if I got everything that needs to be changed for an initial PR.

Closes #4310
First step toward #4683

@DanielNoord DanielNoord added Blocker 🙅 Blocks the next release python 3.7 Maintenance Discussion or action around maintaining pylint or the dev workflow labels Mar 15, 2022
@DanielNoord DanielNoord added this to the 2.14.0 milestone Mar 15, 2022
@coveralls
Copy link

coveralls commented Mar 15, 2022

Pull Request Test Coverage Report for Build 2147752109

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 4 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.03%) to 93.875%

Files with Coverage Reduction New Missed Lines %
pylint/checkers/classes/class_checker.py 1 94.24%
pylint/config/arguments_manager.py 1 89.33%
pylint/checkers/base/name_checker/checker.py 2 98.15%
Totals Coverage Status
Change from base Build 2147474687: -0.03%
Covered Lines: 15878
Relevant Lines: 16914

💛 - Coveralls

@DanielNoord
Copy link
Collaborator Author

With respect to coverage. The first decrease seems "correct", it is no longer covered. Do we want to remove it?

The second coverage decline comes from assign-to-new-keyword become irrelevant. There are no new keywords introduced after 3.7 (match and case are soft keywords), so the message will never be emitted for anything. Again, do we want to remove this? Or just keep it in the codebase without any coverage?

@DanielNoord DanielNoord mentioned this pull request Mar 15, 2022
15 tasks
@DanielNoord DanielNoord marked this pull request as ready for review March 24, 2022 22:58
@DanielNoord DanielNoord requested review from Pierre-Sassoulas and cdce8p and removed request for Pierre-Sassoulas March 24, 2022 22:58
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

We could wait a little for this one to avoid issue with Python 3.6 when backporting fixes on 2.13. What do you think?

@Pierre-Sassoulas
Copy link
Member

I think we should remove dead code no longer accessed.

@DanielNoord
Copy link
Collaborator Author

Fine with me! Shall we say first week of April?

@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Mar 26, 2022

Alternatively we could merge this release 2.14.0 without python 3.6 right now, and move everything that is in 2.14 now to 2.15.0 (then we support 2.14 and python 3.7).

@cdce8p
Copy link
Member

cdce8p commented Mar 26, 2022

I would suggest to wait a bit longer before dropping 3.6, even if it's only in a dev branch. Once it's committed, there sure will be a lot of cleanup and update PRs which will make bugfixes / backports even more difficult. A few weeks more or even a month doesn't make much of a difference IMO.

Alternatively we could merge this release 2.14.0 without python 3.6 right now

Additionally, it would be better not to release new minor versions with so little time after one another. The fast bugfix releases are much appreciated though!

@Pierre-Sassoulas
Copy link
Member

Once it's committed, there sure will be a lot of cleanup and update PRs which will make bugfixes / backports even more difficult.

The idea I had (following #5988 where the test fail for python 3.6) is to not do any bug fixes for python 3.6 / pylint 2.13 and just support pylint 2.14 / python 3.7.

@cdce8p
Copy link
Member

cdce8p commented Mar 26, 2022

Once it's committed, there sure will be a lot of cleanup and update PRs which will make bugfixes / backports even more difficult.

The idea I had (following #5988 where the test fail for python 3.6) is to not do any bug fixes for python 3.6 / pylint 2.13 and just support pylint 2.14 / python 3.7.

🤔 Not sure about that. I would still recommend to wait. I agree pylint 2.14.0 should be the one which requires 3.7, but it would make more sense to include the whole cleanup there as well.

Until we're ready, bugfixes for 2.13.x still need to be tested on 3.6.

After all, the particular issue with 3.6 wasn't a big deal and easy to resolve. It should be possible to support it for a few more weeks now, too.

Copy link
Member

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

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

Not a full review, but from what I've seen this looks about right.

Some notes

@@ -361,12 +361,9 @@ def _check_broken_noreturn(self, node: Union[nodes.Name, nodes.Attribute]) -> No
if (
isinstance(inferred, (nodes.FunctionDef, nodes.ClassDef))
and inferred.qname() in TYPING_NORETURN
# In Python 3.6, NoReturn is alias of '_NoReturn'
# In Python 3.7 - 3.8, NoReturn is alias of '_SpecialForm'
Copy link
Member

Choose a reason for hiding this comment

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

I would leave the second line.

@DanielNoord
Copy link
Collaborator Author

DanielNoord commented Mar 26, 2022

  • We didn't add CI tests for pypy-3.8, right?

No we need to exclude line number checking. Something for another PR :)

I tried, and yeah that's also something for a follow-up 😄 See OP.

@Pierre-Sassoulas
Copy link
Member

we should make sure to update py-version

There's a checklist in #4683 I assumed we were going to do it later on, but doing the whole checklist here would make sense.

@cdce8p
Copy link
Member

cdce8p commented Mar 26, 2022

we should make sure to update py-version

There's a checklist in #4683 I assumed we were going to do it later on, but doing the whole checklist here would make sense.

Not so sure about that. It might be easier to review if they are split up into multiple PRs.

@DanielNoord
Copy link
Collaborator Author

@Pierre-Sassoulas Shall we merge this Monday 11th? We could do one final hotfix this weekend if needed without the trouble of having the hotfix supporting 3.6 and main 3.7. After that we will probably need to be a little more picky when choosing whether something needs to be back ported.

@Pierre-Sassoulas
Copy link
Member

Yes let's do it. I think we can (try to) release 2.14 fairly soon after. This way we don't have to backport to 2.13 and think about python 3.6 anymore

@DanielNoord
Copy link
Collaborator Author

Yes let's do it. I think we can (try to) release 2.14 fairly soon after. This way we don't have to backport to 2.13 and think about python 3.6 anymore

There is quite a lot still on the argparse to-do list, but I agree. We should be able to do 2.14 much sooner than the gap between 2.12 and 2.13. Especially with the speed we have been merging lately.

@DanielNoord DanielNoord removed the Blocker 🙅 Blocks the next release label Apr 11, 2022
@jacobtylerwalls
Copy link
Member

The plan is still to merge this after a likely 2.13.6? Or are we cancelling that milestone?

@DanielNoord
Copy link
Collaborator Author

The plan is still to merge this after a likely 2.13.6? Or are we cancelling that milestone?

I think so, but I think we can already finish the review of the changes here. I'll fix the merge conflict first though.

@DanielNoord DanielNoord added the Blocked 🚧 Blocked by a particular issue label Apr 11, 2022
@Pierre-Sassoulas
Copy link
Member

The plan is still to merge this after a likely 2.13.6?

I need to create a test for the namespace changes but I've been quite busy lately and the main blocker for 2.13.6 is #6141 that needs to be reviewed by Marc ideally. We could release 2.13.6 without this issue in the worst case scenario. And the issue with 3.6 would be manageable anyway. I think we can merge this one.

@Pierre-Sassoulas Pierre-Sassoulas merged commit dfd23f6 into pylint-dev:main Apr 11, 2022
@Pierre-Sassoulas Pierre-Sassoulas removed the Blocked 🚧 Blocked by a particular issue label Apr 11, 2022
@DanielNoord DanielNoord deleted the 3.7 branch April 11, 2022 13:24
@DanielNoord DanielNoord mentioned this pull request Apr 21, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Discussion or action around maintaining pylint or the dev workflow python 3.7
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove support for Python 3.6 and future Python support
5 participants