-
Notifications
You must be signed in to change notification settings - Fork 631
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
Add a .pylintrc & fix warnings #24
Add a .pylintrc & fix warnings #24
Conversation
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.
It is interesting to know that pass statement is not required (and recommended be to removed) when there is a docstring :)
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.
This looks great, and re-disabling checkers when they give us spurious errors seems like the right approach to me.
@@ -68,7 +68,7 @@ | |||
from typing import Iterator | |||
|
|||
|
|||
class Tracer(object): | |||
class Tracer: |
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.
FYI there's a subtle difference here. The old version lets you monkey patch builtins.object
: https://stackoverflow.com/a/45893772. But that seems like a feature we're better off without...
👍 to losing |
This is based on opencensus/opencensus-python#667, with one major difference: I emptied the
disabled
checks and only re-disabled a few select ones. I also commented out a few settings that I believe were equal to the default anyway. Of course, I also fixed the issues pylint already found and added a dev-requirements.txt that can be used to install the right pylint version.One thing I'm not sure about is the "W0107: Unnecessary pass statement (unnecessary-pass)" warning. When a function has a docstring, the pass-statement is not required anymore but I guess you could argue that the resulting code is odd. Still, it removes a few lines so I heeded the warning for now. What do you think?