Flake8 #2804
Flake8 #2804
Conversation
E303 too many blank lines
E111 indentation is not a multiple of four
E117 over-indented
E121 continuation line under-indented for hanging indent
E122 continuation line missing indentation or outdented
E126 continuation line over-indented for hanging indent E127 continuation line over-indented for visual indent E128 continuation line under-indented for visual indent
E203 whitespace before ':' E226 missing whitespace around arithmetic operator E231 missing whitespace after ':' E241 multiple spaces after ':' E261 at least two spaces before inline comment
E265 block comment should start with '# '
E302 expected 2 blank lines, found 1 E305 expected 2 blank lines after class or function definition, found 1
E402 module level import not at top of file
E712 comparison to True should be 'if cond is True:' or 'if cond:' E712 comparison to False should be 'if cond is False:' or 'if not cond:'
W291 trailing whitespace W292 no newline at end of file W293 blank line contains whitespace W391 blank line at end of file
F401 '...' imported but unused F403 'from ... import *' used; unable to detect undefined names
F405 '...' may be undefined, or defined from star imports: ...
F811 redefinition of unused '...'
Codecov Report
@@ Coverage Diff @@
## master #2804 +/- ##
==========================================
- Coverage 52.87% 52.80% -0.08%
==========================================
Files 248 248
Lines 15481 15461 -20
==========================================
- Hits 8186 8164 -22
- Misses 7295 7297 +2 |
F841 local variable '...' is assigned to but never used
|
I've decided to fix W504 (line break after binary operator), E741 (ambiguous variable name '...') and most importantly E501 (line too long (... > 128 characters)). These are a lot of further changes and I had to rewrite lots of url matching tests and also a couple of regular expressions. This needs to be reviewed before it can be merged. |
|
All errors are fixed now. This required fixing some plugins which I initially didn't want to do. In case of F821, these errors were all caused by #1925, which looks like it was a simple replace-string-in-directory job and thus broke lots of stuff. That's kinda bad. It got unnoticed here, as the broken code was unreachable or behind a double geo-block check and thus not executed. Since the current flake8 config doesn't report any problems now with these changes, are we fine with making the lint-check a build success requirement? Open PRs would all have to be rebased once this gets merged. Other PRs which will get merged in the mean time will most likely require me to resolve some merge conflicts then, but I'm fine with that. Also please remember to review these changes before merging (I know it's a lot 😩). Any further fixes/improvements/cleanups will be done in another PR. I'm done here for now unless there are some issues. |
|
@bastimeyer Took me a while but I went through this tonight and it looks good. Maybe we can try for one more review just in case I missed something? I'm also fine with making the lint check a build success requirement, I don't want to review another PR like this in the future. Thanks for doing this. |
W504 line break after binary operator
E741 ambiguous variable name '...'
E501 line too long (... > 128 characters)
F601 dictionary key '...' repeated with different values
E722 do not use bare 'except'
F821 undefined name '...'
|
Fixed the class attributes. |
F812 list comprehension redefines '...' from line ...
|
Interesting... It uses a different set of linting rules when run on Python 2. Didn't know that. Added another commit and reordered again. Should be good now. edit: |
back-to
left a comment
There was a problem hiding this comment.
don't squash the commits when merging
This is an attempt of cleaning up Streamlink's codebase a bit.
I've tried to fix as many errors and warnings as possible, but there's still a lot to do, as I didn't touch those which would require altering the code or reformatting it by a lot (unfortunately Python programmers love silly indentation and code styles). This additional work can be done later in another PR or I'll add some more commits if I feel like.A flake8 config was already added in #2581, but the linting tool was never actually used in the CI runs, so I've enabled that as well (only on TravisCI). It will always succeed though, so if there are any build errors, then it was due to me making mistakes while cleaning up. Some errors/warnings are suppressed via inline comments or in the flake8 config (
setup.cfg).To anyone reviewing the changes, I think it'll be better to go through each commit individually. I've tried to only fix the errors/warnings mentioned in the commit message, but there are also some other minor changes here and there, like deleting an unnecessary empty line between class and method definitions.
Remaining errors: none
There's still more to do than just fixing the reported linting errors/warnings:
unittest.main()calls from test modulesself.loggercalls with nativeloggingmoduleOther things I've noticed:
unittest.TestCase, butunitteststill gets imported (unused import error needs to be suppressed)