Skip to content

Conversation

@back-to
Copy link
Collaborator

@back-to back-to commented Aug 7, 2019

No description provided.

@codecov
Copy link

codecov bot commented Aug 7, 2019

Codecov Report

Merging #2581 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2581   +/-   ##
=======================================
  Coverage   52.86%   52.86%           
=======================================
  Files         239      239           
  Lines       14971    14971           
=======================================
  Hits         7915     7915           
  Misses       7056     7056

1 similar comment
@codecov
Copy link

codecov bot commented Aug 7, 2019

Codecov Report

Merging #2581 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2581   +/-   ##
=======================================
  Coverage   52.86%   52.86%           
=======================================
  Files         239      239           
  Lines       14971    14971           
=======================================
  Hits         7915     7915           
  Misses       7056     7056

venv/,
versioneer.py,
win32/,
max-line-length = 128
Copy link
Member

Choose a reason for hiding this comment

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

👍

@gravyboat
Copy link
Member

I'm good with this. Let's see if anyone else weighs in, mostly @bastimeyer.

@bastimeyer
Copy link
Member

Is the intention of this PR that editors/IDEs can pick up a common linting config? Because that's just the linting config, the actual linter is not being used in the CI.

IIRC, a linter was never added after the fork, because the argument back then was that it was not important at that time. Now, if we add a linting config, then we should also enable it in the CI, so that every PR can be checked.

However, if we add it now, then a lot of work needs to be done first (similar to #414). I'm not sure about the Python-world, but do these linting tools here support automatic code fixing? (I usually don't trust auto-fixing stuff though)

The longer we wait, the more work needs to be done in the future. We could allow linting failures for now though, but I'm not sure how useful it would be, especially when checking new PRs.

$ flake8 --statistics --count --quiet --quiet
3     E117 over-indented
3     E121 continuation line under-indented for hanging indent
1     E122 continuation line missing indentation or outdented
13    E126 continuation line over-indented for hanging indent
3     E127 continuation line over-indented for visual indent
26    E128 continuation line under-indented for visual indent
2     E203 whitespace before ':'
9     E226 missing whitespace around arithmetic operator
18    E241 multiple spaces after ':'
2     E265 block comment should start with '# '
4     E302 expected 2 blank lines, found 1
23    E303 too many blank lines (3)
5     E305 expected 2 blank lines after class or function definition, found 1
3     E402 module level import not at top of file
106   E501 line too long (245 > 128 characters)
1     E711 comparison to None should be 'if cond is not None:'
3     E712 comparison to True should be 'if cond is not True:' or 'if not cond:'
2     E722 do not use bare 'except'
10    E741 ambiguous variable name 'l'
71    F401 '.api.streams' imported but unused
4     F403 'from unittest.mock import *' used; unable to detect undefined names
28    F405 'Stream' may be undefined, or defined from star imports: streamlink.stream
2     F601 dictionary key 'rtmp' repeated with different values
1     F811 redefinition of unused 'test_expired_at' from line 59
5     F821 undefined name 'self'
22    F841 local variable 'e' is assigned to but never used
10    W291 trailing whitespace
1     W292 no newline at end of file
2     W293 blank line contains whitespace
6     W391 blank line at end of file
26    W504 line break after binary operator
415

@back-to
Copy link
Collaborator Author

back-to commented Aug 8, 2019

Is the intention of this PR that editors/IDEs can pick up a common linting config?

yes

then we should also enable it in the CI, so that every PR can be checked.

that could be done in the future, but this PR is just for the editors/IDEs

@beardypig beardypig mentioned this pull request Aug 9, 2019
@gravyboat
Copy link
Member

Are we good to merge this? Is something still holding it up?

@beardypig
Copy link
Member

I'm OK with it.

@gravyboat
Copy link
Member

Merging then. Thanks @back-to!

@gravyboat gravyboat merged commit 7944fab into streamlink:master Sep 6, 2019
@back-to back-to deleted the F8 branch September 8, 2019 18:30
@bastimeyer bastimeyer mentioned this pull request Feb 19, 2020
mkbloke pushed a commit to mkbloke/streamlink that referenced this pull request Aug 18, 2020
setup.cfg: added flake8 settings
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.

4 participants