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

small flake8 changes #318

Merged
merged 4 commits into from
Oct 21, 2015
Merged

small flake8 changes #318

merged 4 commits into from
Oct 21, 2015

Conversation

anarcat
Copy link
Contributor

@anarcat anarcat commented Oct 20, 2015

followup for #4: we configure flake8 to be a little more strict so that new contributions are cleaner. i also fix errors i introduced in recent pull requests, to try to play catchup at least with the new stuff i submitted.

finally, i added a blurb to the development docs about flake8, because i had forgotten about it...

before this PR:

1       E121 continuation line indentation is not a multiple of four
1       E125 continuation line does not distinguish itself from next logical line
25      E127 continuation line over-indented for visual indent
1       E128 continuation line under-indented for visual indent
1       E202 whitespace before ']'
1       E203 whitespace before ','
1       E221 multiple spaces before operator
1       E222 multiple spaces after operator
1       E225 missing whitespace around operator
1       E261 at least two spaces before inline comment
3       E271 multiple spaces after keyword
5       E301 expected 1 blank line, found 0
12      E302 expected 2 blank lines, found 1
1       E303 too many blank lines (2)
12      F401 '__version__' imported but unused
10      F811 redefinition of unused 'cmd' from line 13
4       F812 list comprehension redefines 'action' from line 1463
2       F821 undefined name 'sys'
2       F841 local variable 'e' is assigned to but never used
1       W291 trailing whitespace
3       W293 blank line contains whitespace
4       W391 blank line at end of file
93

(and in the above, i deliberately skipped recursing into .tox, which it does by default now, which triples everything. this PR also fixes that.)

after this PR:

2       E121 continuation line indentation is not a multiple of four
5       E122 continuation line missing indentation or outdented
2       E125 continuation line does not distinguish itself from next logical line
25      E127 continuation line over-indented for visual indent
1       E128 continuation line under-indented for visual indent
1       E202 whitespace before ']'
1       E203 whitespace before ','
1       E221 multiple spaces before operator
1       E222 multiple spaces after operator
1       E225 missing whitespace around operator
1       E261 at least two spaces before inline comment
3       E271 multiple spaces after keyword
5       E301 expected 1 blank line, found 0
10      E302 expected 2 blank lines, found 1
1       E303 too many blank lines (2)
1       E401 multiple imports on one line
62      E501 line too long (149 > 120 characters)
9       F401 'DistutilsOptionError' imported but unused
10      F811 redefinition of unused 'cmd' from line 13
4       F812 list comprehension redefines 'action' from line 1463
3       F821 undefined name 'sys'
2       F841 local variable 'e' is assigned to but never used
1       W291 trailing whitespace
3       W293 blank line contains whitespace
4       W391 blank line at end of file
159

about 30 of those are fixed in #316 as well.

line length comes from @tw - i would prefer 80 columns personnally, but don't want to argue over that bikeshed

the 120 limit *does* trigger warnings right now, but it's better to do that than to make the problem worse for future PRs
@ThomasWaldmann
Copy link
Member

the initial configuration might have been in a way so stuff that is not fixed yet (but present) does not fail all the time, so only new issues are discovered. one can then remove exlusions when fixing the corresponding issues without having it to do all at once (or having it fail all over the place all the time).

@ThomasWaldmann
Copy link
Member

did you notice the travis failure?

@anarcat
Copy link
Contributor Author

anarcat commented Oct 20, 2015

i did not notice the test failure, i'll take a look.

the initial configuration was indeed a way to get flake8 working without too many warnings, but it seems it was too enthusiastic at censoring errors, at least from my perspective... 120 errors is certainly managable, especially since half of those are line length...

@anarcat
Copy link
Contributor Author

anarcat commented Oct 20, 2015

tests should pass now.

@codecov-io
Copy link

Current coverage is 82.84%

Merging #318 into master will decrease coverage by -0.01% as of 52fd8a9

@@            master    #318   diff @@
======================================
  Files           34      34       
  Stmts         7075    7072     -3
  Branches         0       0       
  Methods          0       0       
======================================
- Hit           5862    5859     -3
  Partial          0       0       
  Missed        1213    1213       

Review entire Coverage Diff as of 52fd8a9


Uncovered Suggestions

  1. +0.60% via borg/fuse.py#126...167
  2. +0.48% via borg/xattr.py#199...232
  3. +0.41% via borg/xattr.py#166...194
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

ThomasWaldmann added a commit that referenced this pull request Oct 21, 2015
@ThomasWaldmann ThomasWaldmann merged commit 557d8c7 into borgbackup:master Oct 21, 2015
@anarcat anarcat deleted the flake8 branch October 21, 2015 00:20
This pull request was closed.
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.

3 participants