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.

Sign up for GitHub

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

Jump to bottom

Dev clean lint #36

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Dev clean lint #36

wants to merge 15 commits into from

Conversation

cledouarec
Copy link
Contributor

I have done the cleaning from Pylint output.
I am currently regarding the missing docstring but I will commit it later because it takes time.

I have choiced the default PyCharm line length (120 characters) and it seems to fit well with the current code base.

@cledouarec
Reformat with Black and fix linter errors
c456cec 
Define max line-length to 120 to be compatible with PyCharm default
settings.

Flake 8 :
Ignore E203 whitespace before ':' due to false positive related to slice
notation.
Fix E501 line too long.
Fix F841 local variable 'e' is assigned to but never used.
Fix F401 'StatusType' imported but unused
Fix F841 local variable is assigned to but never used
Fix E712 comparison to False should be 'if cond is False:' or 'if not
cond:'.
Fix E231 missing whitespace after ','

PyLint :
Fix E1101: Instance of 'Exception' has no 'status_code' member
(no-member)
@cledouarec
Fix access to private member of JIRA
b269be7 
Fix PyLint W0212: Access to a protected member _options of a client
class (protected-access)
@cledouarec
Fix default mutable argument
9fcf7f6 
Fix PyLint W0102: Dangerous default value {} as argument (dangerous-default-value)
@cledouarec
Fix shadowing builtin variable name
1ca6a55 
Fix PyLint W0622: Redefining built-in (redefined-builtin)
@cledouarec
Change max line length for PyLint
1a03c83
@cledouarec
Fix lazy log formatting
58ec8b8 
Fix PyLint W1201: Use lazy % formatting in logging functions
(logging-not-lazy)
Fix PyLint W1202: Use lazy % formatting in logging functions
(logging-format-interpolation)
@cledouarec
Fix import order
e2cffd4 
Separate standard module, external module and current module imports.
Reorder in alphabetical order each categories.
Fix PyLint C0411: standard import should be placed before user import
(wrong-import-order)
@cledouarec
Fix deprecated log function
95d3730 
Fix PyLint W1505: Using deprecated method warn() (deprecated-method)
@cledouarec
Clean useless inheritance from object
59711a1 
Fix PyLint R0205: Class class inherits from object, can be safely
removed from bases in python3 (useless-object-inheritance)
@cledouarec
Replace method to static method
ab5fef7 
Fix PyLint R0201: Method could be a function (no-self-use).
@cledouarec
Fix methods that have incoherent return expression
b057bb2 
Fix PyLint R1710: Either all return statements in a function should
return an expression, or none of them should.
(inconsistent-return-statements)
@cledouarec
Fix fixture redefining name from outer scope
26c6cb0 
Fix PyLint W0621: Redefining name from outer scope (redefined-outer-name)

According to Pytest documentation :
If a fixture is used in the same module in which it is defined, the
function name of the fixture will be shadowed by the function arg that
requests the fixture; one way to resolve this is to name the decorated
function fixture_<fixturename> and then use
@pytest.fixture(name='<fixturename>')
@cledouarec
Fix useless else-elif after raise exception
847c61e 
Fix PyLint R1720: Unnecessary "else" after "raise" (no-else-raise)
Fix PyLint R1720: Unnecessary "elif" after "raise" (no-else-raise)
@cledouarec
Fix unused variables
ca2a724 
Fix PyLint W0612: Unused variable (unused-variable)
@cledouarec
Fix variable naming
c913cf0 
Fix PyLint C0103: Variable name doesn't conform to snake_case naming style
(invalid-name)

According to PyLint documentation, the following name are accepted :
i,j,k,ex,Run,_

Use explicit name when possible.
@optilude
Copy link
Contributor

optilude commented Nov 9, 2020

Other than PyLint and I having creative differences about organising imports (which probably don't matter), I'm a worried this is quite a big patch and will cause other live branches a lot of rebase type problems.

Wondering if we could do the following:

  • Disable the linter rule that demands double-quotes instead of single-quotes everywhere. I actually really disagree with this, but maybe that's because I'm old and stuck in my ways. But I think it accounts for a huge amount of lines changed and probably doesn't really add anything in terms of readability or catching errors.
  • Less critical, but I think sometimes forcing a multi-lined import or list into a single line makes it less readable and harder to work with (not least because a diff will operate only on line level so if you change one item in the list it looks like everything's changed)
  • Conversely, in the tests there are cases where we've split some comparison lists/dicts into multiple lines and this actually makes it much harder to see what's being tested for. It's important that you can see the whole assert in one place and that things line up to make it easier to spot the (often quite subtle) differences between test cases. So maybe there's just a set of rules about single/multiple lines that ought to be turned off and then leave everything as it was.

I hope this doesn't create a ton of new work. I'm really grateful for the PR and the work going into improving code quality!

@cledouarec
Copy link
Contributor Author

I'm pretty sure most of the works will rebase fine 😃
For your 3 points, I must change Black formatter that has a fixed configuration to yapf that could be customizing more deeply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cledouarec @optilude