add linting to our test suite / CI

[anarcat]

what i learned from #398 is that we are missing some linting steps that were previously somewhat part of the development process from CI and the test suite.

we could, for example, run flake8 from tox.ini (which would hook it into CI, presumably), and also twine check for... er.... whatever that does.

we also discussed just throwing the entire thing into black, but that seems like a rather bold move at this stage, i would start slow unless we're totally flake8-broken and it would be easier to let black do the hard work.

[mgedmin]

Well, currently flake8 finds these errors:

1     E111 indentation is not a multiple of four
1     E114 indentation is not a multiple of four (comment)
2     E122 continuation line missing indentation or outdented
3     E124 closing bracket does not match visual indentation
1     E125 continuation line with same indent as next logical line
21    E127 continuation line over-indented for visual indent
137   E128 continuation line under-indented for visual indent
2     E129 visually indented line with same indent as next logical line
4     E131 continuation line unaligned for hanging indent
1     E201 whitespace after '['
1     E202 whitespace before ']'
4     E203 whitespace before ','
7     E221 multiple spaces before operator
2     E222 multiple spaces after operator
31    E225 missing whitespace around operator
1     E228 missing whitespace around modulo operator
10    E231 missing whitespace after ','
46    E251 unexpected spaces around keyword / parameter equals
40    E261 at least two spaces before inline comment
12    E265 block comment should start with '# '
5     E266 too many leading '#' for block comment
2     E271 multiple spaces after keyword
2     E301 expected 1 blank line, found 0
71    E302 expected 2 blank lines, found 1
1     E303 too many blank lines (3)
6     E305 expected 2 blank lines after class or function definition, found 1
7     E306 expected 1 blank line before a nested definition, found 0
31    E402 module level import not at top of file
191   E501 line too long (110 > 80 characters)
24    E502 the backslash is redundant between brackets
32    E701 multiple statements on one line (colon)
4     E722 do not use bare 'except'
6     E731 do not assign a lambda expression, use a def
6     E741 ambiguous variable name 'l'
11    F401 '.logconf.LOG_ROOT' imported but unused
25    F821 undefined name 'unicode'
182   W191 indentation contains tabs
6     W291 trailing whitespace
1     W293 blank line contains whitespace
5     W391 blank line at end of file

Those 25 F821 undefined name 'unicode' errors I'm inclined to fix by hand, unless @cjmayo beats me to it. The rest... maybe we should let black loose?

[anarcat]

The rest... maybe we should let black loose?

happy either way, but i'd like us to get flake8-clean. and i don't mean PEP8 clean here: we can set our own exceptions and so on, but we should explicitely make those.

[cjmayo]

Those 25 F821 undefined name 'unicode' errors I'm inclined to fix by hand, unless @cjmayo beats me to it.

Go for it! Those were the potential crashers I mentioned in the other issue. I was thinking about leaving the ones in the Windows only code - unless you are confident about how they should be fixed - marking them with #noqa and then providing the flake8 command to find them in the Windows call for help.

The rest... maybe we should let black loose?

It is tempting. I'm a bit put off when the maintainers still call it beta. But we have so much it is probably worth the risk - else we will have to create a whole load of exclusions and not catch those in new code.

I still have one PR's worth of existing commits to submit though, if we can hold off a bit.

[anarcat]

I still have one PR's worth of existing commits to submit though, if we can hold off a bit.

happy to. no rush. linting should never be prioritized over bugfixes and other cleanups :)

[anarcat]

i have found this review which evaluates meta-tools to hook up various linting tools:

https://blog.urth.org/2020/05/08/comparing-code-quality-meta-tools/

we could consider it to setup local (ie. "pre-commit") and remote (ie. CI) hooks for liniting... or do we just want to rely on tox for that?

[cjmayo]

We've got a fair bit in the GitHub workflow now, make check collects some local tools together including flake8.