Borg readability and coding style

[maltefiala]

I am not really fond of the readability of attic's code. Variable names like "t0" and "st" are nice to write but don't make it very readable and brake PEP 0008 as they clearly aren't words:

lowercase with words separated by underscores as necessary to improve readability.

However, changing variable names should be discussed as it brakes code compatibility with attic, see #1

[anarcat]

i don't think code compatibility with attic is such a big issue as on-disk format compatibility.

[ThomasWaldmann]

I added configuration so we are able to run flake8 code style checker locally.
The current configuration is made so it does not trigger any warnings/errors for stuff that is already present in existing code. Later (NOT NOW), when all the merges are done, we can improve style by reducing the excludes in setup.cfg flake8 section and at the same time fixing the style.

[ThomasWaldmann]

some stuff reported by flake8 was fixed here (I first checked that it is just minor stuff, not likely to cause merge issues):
4b81f38

[anarcat]

for future reference, the configuration is done in setup.cfg:

[flake8]
ignore = E226,F403
max-line-length = 250
exclude = docs/conf.py,borg/_version.py,build,dist,.git,.idea,.cache,.tox
max-complexity = 100

F403 is: "from module import * used; unable to detect undefined names"
F226 is: "missing whitespace around arithmetic operator"

i believe @ThomasWaldmann was saying that the line limit is 120 characters (and not 250). max complexity, i have no idea what it's for.

even with the above checks skipped, we have 62 messages from flake8:

$ python3 /usr/lib/python3/dist-packages/flake8/__main__.py
./setup.py:9:1: F401 'DistutilsOptionError' imported but unused
./setup.py:25:17: E225 missing whitespace around operator
./setup.py:77:9: E125 continuation line does not distinguish itself from next logical line
./setup.py:102:121: E501 line too long (149 > 120 characters)
./setup.py:107:121: E501 line too long (124 > 120 characters)
./setup.py:126:1: E302 expected 2 blank lines, found 1
./setup.py:132:5: E301 expected 1 blank line, found 0
./setup.py:183:5: E301 expected 1 blank line, found 0
./setup.py:246:5: E122 continuation line missing indentation or outdented
./setup.py:246:121: E501 line too long (123 > 120 characters)
./setup.py:247:5: E122 continuation line missing indentation or outdented
./setup.py:247:121: E501 line too long (122 > 120 characters)
./setup.py:248:5: E122 continuation line missing indentation or outdented
./setup.py:249:5: E122 continuation line missing indentation or outdented
./setup.py:250:1: E122 continuation line missing indentation or outdented
./docs/conf.py:17:11: E401 multiple imports on one line
./docs/conf.py:190:3: E121 continuation line indentation is not a multiple of four
./docs/conf.py:227:121: E501 line too long (123 > 120 characters)
./borg/__init__.py:3:1: F401 '__version__' imported but unused
./borg/__main__.py:3:1: W391 blank line at end of file
./borg/archive.py:21:121: E501 line too long (121 > 120 characters)
./borg/archive.py:136:121: E501 line too long (128 > 120 characters)
./borg/archive.py:586:121: E501 line too long (144 > 120 characters)
./borg/archive.py:757:121: E501 line too long (180 > 120 characters)
./borg/archiver.py:18:1: F401 'format_timedelta' imported but unused
./borg/archiver.py:195:13: E125 continuation line does not distinguish itself from next logical line
./borg/archiver.py:252:121: E501 line too long (125 > 120 characters)
./borg/archiver.py:289:55: E271 multiple spaces after keyword
./borg/archiver.py:322:121: E501 line too long (129 > 120 characters)
./borg/archiver.py:562:121: E501 line too long (130 > 120 characters)
./borg/archiver.py:564:121: E501 line too long (125 > 120 characters)
./borg/archiver.py:695:121: E501 line too long (139 > 120 characters)
./borg/archiver.py:762:121: E501 line too long (148 > 120 characters)
./borg/cache.py:4:1: F401 'errno' imported but unused
./borg/cache.py:10:1: F401 'tarfile' imported but unused
./borg/cache.py:11:1: F401 'tempfile' imported but unused
./borg/cache.py:62:121: E501 line too long (157 > 120 characters)
./borg/cache.py:90:121: E501 line too long (140 > 120 characters)
./borg/cache.py:141:15: E271 multiple spaces after keyword
./borg/cache.py:144:9: F841 local variable 'e' is assigned to but never used
./borg/fuse.py:46:121: E501 line too long (121 > 120 characters)
./borg/helpers.py:173:121: E501 line too long (145 > 120 characters)
./borg/helpers.py:479:121: E501 line too long (143 > 120 characters)
./borg/helpers.py:483:121: E501 line too long (135 > 120 characters)
./borg/key.py:200:60: F821 undefined name 'sys'
./borg/key.py:202:60: F821 undefined name 'sys'
./borg/locking.py:99:25: E221 multiple spaces before operator
./borg/logger.py:36:1: F401 'sys' imported but unused
./borg/remote.py:137:67: E202 whitespace before ']'
./borg/repository.py:11:1: F401 'sys' imported but unused
./borg/repository.py:113:121: E501 line too long (127 > 120 characters)
./borg/repository.py:347:121: E501 line too long (128 > 120 characters)
./borg/repository.py:602:121: E501 line too long (123 > 120 characters)
./borg/repository.py:608:121: E501 line too long (123 > 120 characters)
./borg/xattr.py:69:121: E501 line too long (121 > 120 characters)
./borg/support/__init__.py:16:1: W391 blank line at end of file
./borg/support/argparse.py:97:5: E301 expected 1 blank line, found 0
./borg/support/argparse.py:115:1: E302 expected 2 blank lines, found 1
./borg/support/argparse.py:624:44: E127 continuation line over-indented for visual indent
./borg/support/argparse.py:677:15: E271 multiple spaces after keyword
./borg/support/argparse.py:1159:1: E302 expected 2 blank lines, found 1
./borg/support/argparse.py:1263:5: E303 too many blank lines (2)
./borg/support/argparse.py:1468:57: F812 list comprehension redefines 'action' from line 1463
./borg/support/argparse.py:2092:17: E128 continuation line under-indented for visual indent
./borg/support/argparse.py:2092:21: F812 list comprehension redefines 'action' from line 2082
./borg/support/argparse.py:2092:29: F812 list comprehension redefines 'option_string' from line 2080
./borg/support/argparse.py:2092:44: F812 list comprehension redefines 'explicit_arg' from line 2080
./borg/testsuite/archiver.py:74:1: E302 expected 2 blank lines, found 1
./borg/testsuite/archiver.py:120:5: E301 expected 1 blank line, found 0
./borg/testsuite/archiver.py:211:121: E501 line too long (127 > 120 characters)
./borg/testsuite/archiver.py:238:19: E222 multiple spaces after operator
./borg/testsuite/archiver.py:257:46: E261 at least two spaces before inline comment
./borg/testsuite/archiver.py:321:42: F841 local variable 'fd' is assigned to but never used
./borg/testsuite/archiver.py:343:121: E501 line too long (138 > 120 characters)
./borg/testsuite/archiver.py:356:121: E501 line too long (147 > 120 characters)
./borg/testsuite/archiver.py:392:121: E501 line too long (124 > 120 characters)
./borg/testsuite/archiver.py:765:121: E501 line too long (130 > 120 characters)
./borg/testsuite/benchmark.py:13:1: F401 'cmd' imported but unused
./borg/testsuite/benchmark.py:27:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:53:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:60:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:65:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:70:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:76:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:81:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:86:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:91:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:97:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:100:1: W391 blank line at end of file
./borg/testsuite/chunker.py:16:121: E501 line too long (188 > 120 characters)
./borg/testsuite/chunker.py:17:121: E501 line too long (203 > 120 characters)
./borg/testsuite/chunker.py:18:121: E501 line too long (193 > 120 characters)
./borg/testsuite/chunker.py:19:121: E501 line too long (143 > 120 characters)
./borg/testsuite/chunker.py:20:121: E501 line too long (188 > 120 characters)
./borg/testsuite/chunker.py:21:121: E501 line too long (188 > 120 characters)
./borg/testsuite/chunker.py:22:121: E501 line too long (143 > 120 characters)
./borg/testsuite/chunker.py:23:121: E501 line too long (178 > 120 characters)
./borg/testsuite/chunker.py:24:121: E501 line too long (173 > 120 characters)
./borg/testsuite/chunker.py:29:121: E501 line too long (134 > 120 characters)
./borg/testsuite/compress.py:102:1: W391 blank line at end of file
./borg/testsuite/helpers.py:11:121: E501 line too long (148 > 120 characters)
./borg/testsuite/helpers.py:187:121: E501 line too long (125 > 120 characters)
./borg/testsuite/helpers.py:384:121: E501 line too long (128 > 120 characters)
./borg/testsuite/key.py:51:121: E501 line too long (122 > 120 characters)
./borg/testsuite/key.py:66:121: E501 line too long (129 > 120 characters)
./borg/testsuite/key.py:85:121: E501 line too long (121 > 120 characters)
./borg/testsuite/key.py:96:121: E501 line too long (132 > 120 characters)
./borg/testsuite/key.py:102:121: E501 line too long (122 > 120 characters)
./borg/testsuite/locking.py:5:58: E203 whitespace before ','
./borg/testsuite/locking.py:11:1: E302 expected 2 blank lines, found 1
./borg/testsuite/platform.py:60:121: E501 line too long (151 > 120 characters)
./borg/testsuite/platform.py:65:121: E501 line too long (151 > 120 characters)
./borg/testsuite/platform.py:135:121: E501 line too long (190 > 120 characters)
./borg/testsuite/platform.py:136:121: E501 line too long (131 > 120 characters)
./borg/testsuite/platform.py:137:121: E501 line too long (128 > 120 characters)
./borg/testsuite/platform.py:138:121: E501 line too long (190 > 120 characters)
./borg/testsuite/platform.py:139:121: E501 line too long (131 > 120 characters)
./borg/testsuite/platform.py:140:121: E501 line too long (146 > 120 characters)
./borg/testsuite/repository.py:198:121: E501 line too long (138 > 120 characters)
./borg/testsuite/repository.py:211:121: E501 line too long (123 > 120 characters)
./borg/testsuite/repository.py:330:121: E501 line too long (141 > 120 characters)
./borg/testsuite/repository.py:331:121: E501 line too long (147 > 120 characters)
./borg/testsuite/repository.py:332:121: E501 line too long (157 > 120 characters)
./borg/testsuite/repository.py:333:121: E501 line too long (176 > 120 characters)
./borg/testsuite/repository.py:335:121: E501 line too long (150 > 120 characters)
./borg/testsuite/upgrader.py:25:18: F821 undefined name 'Repository'
[1056]anarcat@marcos:borg1$ python3 /usr/lib/python3/dist-packages/flake8/__main__.py
./setup.py:9:1: F401 'DistutilsOptionError' imported but unused
./setup.py:25:17: E225 missing whitespace around operator
./setup.py:77:9: E125 continuation line does not distinguish itself from next logical line
./setup.py:126:1: E302 expected 2 blank lines, found 1
./setup.py:132:5: E301 expected 1 blank line, found 0
./setup.py:183:5: E301 expected 1 blank line, found 0
./setup.py:246:5: E122 continuation line missing indentation or outdented
./setup.py:247:5: E122 continuation line missing indentation or outdented
./setup.py:248:5: E122 continuation line missing indentation or outdented
./setup.py:249:5: E122 continuation line missing indentation or outdented
./setup.py:250:1: E122 continuation line missing indentation or outdented
./docs/conf.py:17:11: E401 multiple imports on one line
./docs/conf.py:190:3: E121 continuation line indentation is not a multiple of four
./borg/__init__.py:3:1: F401 '__version__' imported but unused
./borg/__main__.py:3:1: W391 blank line at end of file
./borg/archiver.py:18:1: F401 'format_timedelta' imported but unused
./borg/archiver.py:195:13: E125 continuation line does not distinguish itself from next logical line
./borg/archiver.py:289:55: E271 multiple spaces after keyword
./borg/cache.py:4:1: F401 'errno' imported but unused
./borg/cache.py:10:1: F401 'tarfile' imported but unused
./borg/cache.py:11:1: F401 'tempfile' imported but unused
./borg/cache.py:141:15: E271 multiple spaces after keyword
./borg/cache.py:144:9: F841 local variable 'e' is assigned to but never used
./borg/key.py:200:60: F821 undefined name 'sys'
./borg/key.py:202:60: F821 undefined name 'sys'
./borg/locking.py:99:25: E221 multiple spaces before operator
./borg/logger.py:36:1: F401 'sys' imported but unused
./borg/remote.py:137:67: E202 whitespace before ']'
./borg/repository.py:11:1: F401 'sys' imported but unused
./borg/support/__init__.py:16:1: W391 blank line at end of file
./borg/support/argparse.py:97:5: E301 expected 1 blank line, found 0
./borg/support/argparse.py:115:1: E302 expected 2 blank lines, found 1
./borg/support/argparse.py:624:44: E127 continuation line over-indented for visual indent
./borg/support/argparse.py:677:15: E271 multiple spaces after keyword
./borg/support/argparse.py:1159:1: E302 expected 2 blank lines, found 1
./borg/support/argparse.py:1263:5: E303 too many blank lines (2)
./borg/support/argparse.py:1468:57: F812 list comprehension redefines 'action' from line 1463
./borg/support/argparse.py:2092:17: E128 continuation line under-indented for visual indent
./borg/support/argparse.py:2092:21: F812 list comprehension redefines 'action' from line 2082
./borg/support/argparse.py:2092:29: F812 list comprehension redefines 'option_string' from line 2080
./borg/support/argparse.py:2092:44: F812 list comprehension redefines 'explicit_arg' from line 2080
./borg/testsuite/archiver.py:74:1: E302 expected 2 blank lines, found 1
./borg/testsuite/archiver.py:120:5: E301 expected 1 blank line, found 0
./borg/testsuite/archiver.py:238:19: E222 multiple spaces after operator
./borg/testsuite/archiver.py:257:46: E261 at least two spaces before inline comment
./borg/testsuite/archiver.py:321:42: F841 local variable 'fd' is assigned to but never used
./borg/testsuite/benchmark.py:13:1: F401 'cmd' imported but unused
./borg/testsuite/benchmark.py:27:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:53:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:60:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:65:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:70:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:76:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:81:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:86:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:91:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:97:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:100:1: W391 blank line at end of file
./borg/testsuite/compress.py:102:1: W391 blank line at end of file
./borg/testsuite/locking.py:5:58: E203 whitespace before ','
./borg/testsuite/locking.py:11:1: E302 expected 2 blank lines, found 1
./borg/testsuite/upgrader.py:25:18: F821 undefined name 'Repository'
[1056]anarcat@marcos:borg1$ python3 /usr/lib/python3/dist-packages/flake8/__main__.py --stat --count
./setup.py:9:1: F401 'DistutilsOptionError' imported but unused
./setup.py:25:17: E225 missing whitespace around operator
./setup.py:77:9: E125 continuation line does not distinguish itself from next logical line
./setup.py:126:1: E302 expected 2 blank lines, found 1
./setup.py:132:5: E301 expected 1 blank line, found 0
./setup.py:183:5: E301 expected 1 blank line, found 0
./setup.py:246:5: E122 continuation line missing indentation or outdented
./setup.py:247:5: E122 continuation line missing indentation or outdented
./setup.py:248:5: E122 continuation line missing indentation or outdented
./setup.py:249:5: E122 continuation line missing indentation or outdented
./setup.py:250:1: E122 continuation line missing indentation or outdented
./docs/conf.py:17:11: E401 multiple imports on one line
./docs/conf.py:190:3: E121 continuation line indentation is not a multiple of four
./borg/__init__.py:3:1: F401 '__version__' imported but unused
./borg/__main__.py:3:1: W391 blank line at end of file
./borg/archiver.py:18:1: F401 'format_timedelta' imported but unused
./borg/archiver.py:195:13: E125 continuation line does not distinguish itself from next logical line
./borg/archiver.py:289:55: E271 multiple spaces after keyword
./borg/cache.py:4:1: F401 'errno' imported but unused
./borg/cache.py:10:1: F401 'tarfile' imported but unused
./borg/cache.py:11:1: F401 'tempfile' imported but unused
./borg/cache.py:141:15: E271 multiple spaces after keyword
./borg/cache.py:144:9: F841 local variable 'e' is assigned to but never used
./borg/key.py:200:60: F821 undefined name 'sys'
./borg/key.py:202:60: F821 undefined name 'sys'
./borg/locking.py:99:25: E221 multiple spaces before operator
./borg/logger.py:36:1: F401 'sys' imported but unused
./borg/remote.py:137:67: E202 whitespace before ']'
./borg/repository.py:11:1: F401 'sys' imported but unused
./borg/support/__init__.py:16:1: W391 blank line at end of file
./borg/support/argparse.py:97:5: E301 expected 1 blank line, found 0
./borg/support/argparse.py:115:1: E302 expected 2 blank lines, found 1
./borg/support/argparse.py:624:44: E127 continuation line over-indented for visual indent
./borg/support/argparse.py:677:15: E271 multiple spaces after keyword
./borg/support/argparse.py:1159:1: E302 expected 2 blank lines, found 1
./borg/support/argparse.py:1263:5: E303 too many blank lines (2)
./borg/support/argparse.py:1468:57: F812 list comprehension redefines 'action' from line 1463
./borg/support/argparse.py:2092:17: E128 continuation line under-indented for visual indent
./borg/support/argparse.py:2092:21: F812 list comprehension redefines 'action' from line 2082
./borg/support/argparse.py:2092:29: F812 list comprehension redefines 'option_string' from line 2080
./borg/support/argparse.py:2092:44: F812 list comprehension redefines 'explicit_arg' from line 2080
./borg/testsuite/archiver.py:74:1: E302 expected 2 blank lines, found 1
./borg/testsuite/archiver.py:120:5: E301 expected 1 blank line, found 0
./borg/testsuite/archiver.py:238:19: E222 multiple spaces after operator
./borg/testsuite/archiver.py:257:46: E261 at least two spaces before inline comment
./borg/testsuite/archiver.py:321:42: F841 local variable 'fd' is assigned to but never used
./borg/testsuite/benchmark.py:13:1: F401 'cmd' imported but unused
./borg/testsuite/benchmark.py:27:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:53:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:60:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:65:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:70:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:76:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:81:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:86:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:91:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:97:1: F811 redefinition of unused 'cmd' from line 13
./borg/testsuite/benchmark.py:100:1: W391 blank line at end of file
./borg/testsuite/compress.py:102:1: W391 blank line at end of file
./borg/testsuite/locking.py:5:58: E203 whitespace before ','
./borg/testsuite/locking.py:11:1: E302 expected 2 blank lines, found 1
./borg/testsuite/upgrader.py:25:18: F821 undefined name 'Repository'
1       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
1       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
4       E301 expected 1 blank line, found 0
5       E302 expected 2 blank lines, found 1
1       E303 too many blank lines (2)
1       E401 multiple imports on one line
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
4       W391 blank line at end of file
62

i think we should remove the ignores or complexity limitations, because they don't change the error count. lowering the line length to 120 chars adds another 64 warnings.

it seems to me this should be intergrated in our CI workflow, otherwise we'll always be catching up to this. a bit like codecov, but without the emails at each PR. :P

[anarcat]

we should also write up a blurb about coding style in development.rst, esp. for those who don't know (or forgot, like me) about flake8.

[anarcat]

config fixed and docs done in #318, next step is to do the actual cleanup - not much to be done actually, and hook into CI/github (not sure how).

[anarcat]

another todo here, from #3, is to take a look at the codeclimate stuff:

• https://codeclimate.com/github/jborg/attic/code
• https://codeclimate.com/github/borgbackup/borg

(merged #3 here)

[ThomasWaldmann]

add automated pep8/flakes test after 1.0.