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

Remove unnecessary base class and dead code in pylint.pyreverse.utils #6712

Merged
merged 2 commits into from
May 27, 2022

Conversation

DudeNr33
Copy link
Collaborator

@DudeNr33 DudeNr33 commented May 27, 2022

  • 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.
  • If you used multiple emails or multiple names when contributing, add your mails
    and preferred name in script/.contributors_aliases.json

Type of Changes

Type
🔨 Refactoring

Description

Ref #6582
Closes the first bullet point from the list:
ASTWalker and LocalsVisitor were apparently moved from astroid.utils to pylint.pyreverse.utils some years ago.
Maybe originally there were also other subclasses of ASTWalker, but at least in pyreverse that is not the case.

Thus we can remove ASTWalker and some of the unused code in there and directly put the remaining stuff in LocalsVisitor.

Some remarks, as the diff is a bit hard to read imo:

  • handler was always self -> removed handler and replaced references with self
  • walk and leave were never called -> removed

@DudeNr33 DudeNr33 added pyreverse Related to pyreverse component Maintenance Discussion or action around maintaining pylint or the dev workflow labels May 27, 2022
@DudeNr33 DudeNr33 added this to the 2.15.0 milestone May 27, 2022
@DudeNr33
Copy link
Collaborator Author

@Pierre-Sassoulas regarding your idea of user facing / dev related changes in #6632 (comment):
Do you think I should add a Changelog entry somewhere?

@coveralls
Copy link

coveralls commented May 27, 2022

Pull Request Test Coverage Report for Build 2394816447

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.09%) to 95.513%

Totals Coverage Status
Change from base Build 2392933967: 0.09%
Covered Lines: 16221
Relevant Lines: 16983

💛 - Coveralls

@Pierre-Sassoulas
Copy link
Member

@Pierre-Sassoulas regarding your idea of user facing / dev related changes in #6632 (comment): Do you think I should add a Changelog entry somewhere?

We'll do the switch for 2.15, let's keep on doing it like we were for 2.14.

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.

LGTM, I think it can go in 2.14 even without changelog, it's an internal refactor, right ?

@DudeNr33
Copy link
Collaborator Author

DudeNr33 commented May 27, 2022

Yes, this should be purely internal. At least I would not say the ASTWalker class is considered public API, and there is no plugin system for pyreverse.

I did a GitHub search and only found one instance where it was actually used, but that was just part of a demo talk in 2017.

I'll add a ChangeLog entry anyway just so that any potential users could find some information about it. Doesn't cost much and might prevent questions, although I don't really expect any problems.

@DudeNr33 DudeNr33 modified the milestones: 2.15.0, 2.14.0 May 27, 2022
@DudeNr33 DudeNr33 merged commit b454b12 into pylint-dev:main May 27, 2022
DudeNr33 added a commit to DudeNr33/pylint that referenced this pull request May 27, 2022
DudeNr33 added a commit that referenced this pull request May 27, 2022
…everse` (#6713)

* Remove variables and code paths that are always static in productive use.

* Add deprecation warning if an interface defined through ``__implements__`` is found.

* Update pylint/pyreverse/inspector.py

Co-authored-by: Pierre Sassoulas <[email protected]>

* Group changelog entries together with refactor from #6712

Co-authored-by: Pierre Sassoulas <[email protected]>
@DudeNr33 DudeNr33 deleted the refactor-pyreverse-visitor branch May 27, 2022 18:41
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 pyreverse Related to pyreverse component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants