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

Create new error arguments-renamed #4467

Merged
merged 11 commits into from
May 17, 2021

Conversation

ksaketou
Copy link
Contributor

Steps

  • Add yourself to CONTRIBUTORS 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.

Description

This pull request creates a new error called arguments-renamed, which identifies any changes in the
parameter names of overridden functions. It separates the functionality of arguments-differ.
Based on additions of #4422 and #4456

It also merges the two kinds of test files (Py3- and Py3+) that existed for arguments-differ in one.

Type of Changes

Type
✨ New feature
🔨 Refactoring

Related Issue

Part two and closes #3536

ksaketou and others added 7 commits May 11, 2021 14:21
This commits creates the new error arguments-renamed based on the functionality
added on pylint-dev#4422 and the changes of pylint-dev#4456.
This commit merges the two kinds of test files that existed for the arguments-differ error, since now all
tests run in Python3.
@coveralls
Copy link

coveralls commented May 11, 2021

Coverage Status

Coverage increased (+0.005%) to 91.717% when pulling bf9ebd6 on ksaketou:args-renamed into 18ef7ba on PyCQA:master.

@ksaketou
Copy link
Contributor Author

@Pierre-Sassoulas what do you think about this? :)

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.

Looks good, thanks. Lots of test added, very few changes, I like that :) ! I just have a small remark to reduce duplication. Maybe you want to take a look too @cdce8p ?

pylint/checkers/classes.py Show resolved Hide resolved
@cdce8p
Copy link
Member

cdce8p commented May 15, 2021

Maybe you want to take a look too @cdce8p ?

@Pierre-Sassoulas I have only briefly looked at it. I don't think it will add any errors so it should be ok once the review process is done.

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.

Thanks for the refactor ! And thank for following through with this new message !

@Pierre-Sassoulas Pierre-Sassoulas merged commit d150e39 into pylint-dev:master May 17, 2021
@ksaketou
Copy link
Contributor Author

My pleasure! I'm glad I helped :)

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.

arguments-differ is triggered if just arg names changed
4 participants