Skip to content

Fix new pylint errors#18190

Merged
gnossen merged 2 commits intomasterfrom
new-pylint-errors
Feb 27, 2019
Merged

Fix new pylint errors#18190
gnossen merged 2 commits intomasterfrom
new-pylint-errors

Conversation

@gnossen
Copy link
Copy Markdown
Contributor

@gnossen gnossen commented Feb 27, 2019

Resolves #18189. An updated transitive linter dependency has recently made our linter more strict. The main new finding is that a couple of our classes have 8 ancestors, where it believes 7 is the magic number.

There are three approaches to this problem:

  1. Ignore the warning on these particular lines.
  2. Increase the maximum allowable number of ancestors.
  3. Change the hierarchy.

Option 3 is a non-starter. I opted for option 1 because we're not tacitly granting approval; we're recognizing that this is a problem

@gnossen gnossen added lang/Python release notes: no Indicates if PR should not be in release notes labels Feb 27, 2019
@gnossen gnossen requested a review from lidizheng February 27, 2019 17:52
Copy link
Copy Markdown
Contributor

@lidizheng lidizheng left a comment

Choose a reason for hiding this comment

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

Do you think we should ignore this rule altogether in .pylintrc?

return self._traceback

def add_callback(self, callback):
del callback
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Test code here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@gnossen gnossen merged commit b558519 into master Feb 27, 2019
apolcyn added a commit to apolcyn/grpc that referenced this pull request Mar 5, 2019
apolcyn added a commit that referenced this pull request Mar 5, 2019
Fix pylint on 1.19.x - backport #18190
@gnossen gnossen deleted the new-pylint-errors branch May 6, 2019 23:50
@lock lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lang/Python release notes: no Indicates if PR should not be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pylint sanity test is broken on master

3 participants