Outside any header requirements or directive documentation
requirements,
are there any changes to PEP8 that folks want to make? If so, please
list the exceptions and why you think they should be adopted.
==== E251 =====
libtaskotron/directives/bodhi_comment_directive.py:247:67: E251 unexpected spaces around
keyword / parameter equals
libtaskotron/directives/bodhi_comment_directive.py:247:69: E251 unexpected spaces around
keyword / parameter equals
libtaskotron/directives/bodhi_comment_directive.py:293:13: E251 unexpected spaces around
keyword / parameter equals
libtaskotron/directives/bodhi_comment_directive.py:293:15: E251 unexpected spaces around
keyword / parameter equals
libtaskotron/directives/bodhi_comment_directive.py:293:31: E251 unexpected spaces around
keyword / parameter equals
libtaskotron/directives/bodhi_comment_directive.py:293:33: E251 unexpected spaces around
keyword / parameter equals
libtaskotron/directives/bodhi_comment_directive.py:426:28: E251 unexpected spaces around
keyword / parameter equals
libtaskotron/directives/bodhi_comment_directive.py:426:30: E251 unexpected spaces around
keyword / parameter equals
libtaskotron/directives/bodhi_comment_directive.py:427:33: E251 unexpected spaces around
keyword / parameter equals
libtaskotron/directives/bodhi_comment_directive.py:427:35: E251 unexpected spaces around
keyword / parameter equals
libtaskotron/directives/bodhi_comment_directive.py:428:29: E251 unexpected spaces around
keyword / parameter equals
libtaskotron/directives/bodhi_comment_directive.py:428:31: E251 unexpected spaces around
keyword / parameter equals
def _is_comment_needed(old_result, comment_time, result, time_span = None):
-or-
outcome = _post_testresult(self.bodhi_api, detail.item,
env_data['checkname'],
detail.outcome,
url = "http://example.com",
doreport = input_data['doreport'],
arch = detail.keyvals.get('arch', 'noarch'),
)
Josef is used to add spaces between keyword name and its value in method definitions or
method calls. Personally, I find it more readable according to PEP8 (with no space), but
Josef claims the opposite :)
==== E128 ====
libtaskotron/check.py:94:17: E128 continuation line under-indented for visual indent
libtaskotron/check.py:97:21: E128 continuation line under-indented for visual indent
libtaskotron/check.py:209:21: E128 continuation line under-indented for visual indent
raise exc.TaskotronValueError("'output' parameter must be a
"
"mutable sequence of strings. Yours was: %s" % type(output))
This is a braindead check. You have a limited line length, and it is happens that you need
to type the opening bracket shortly before the end of the line, PEP8 imagines you end up
providing the rest of the code as a short column of text underneath it. Like this:
raise exc.TaskotronValueError("'output' parameter must be a
"
"mutable sequence of strings. "
"Yours was: %s" % type(output))
or you break the line immediately after the opening bracket, like this:
raise exc.TaskotronValueError(
"'output' parameter must be a "
"mutable sequence of strings. Yours was: %s" % type(output))
Still, the most readable code I believe is the first one, against PEP8.
==== E303 ====
libtaskotron/check.py:88:5: E303 too many blank lines (2)
libtaskotron/check.py:110:5: E303 too many blank lines (2)
libtaskotron/check.py:115:5: E303 too many blank lines (2)
libtaskotron/check.py:122:5: E303 too many blank lines (2)
libtaskotron/check.py:142:5: E303 too many blank lines (2)
libtaskotron/check.py:148:5: E303 too many blank lines (2)
libtaskotron/check.py:165:5: E303 too many blank lines (2)
libtaskotron/check.py:189:5: E303 too many blank lines (2)
libtaskotron/check.py:224:5: E303 too many blank lines (2)
This forbids you from adding two blank lines between class methods. Another braindead
checks.
==== E124 ====
libtaskotron/bodhi_utils.py:146:25: E124 closing bracket does not match visual
indentation
log.warn("The build %s is a part of two different updates:
'%s'"
" and '%s'. That's probably a bug in
Bodhi." % (build,
failures[build]['title'],
build2update[build]['title'])
)
Because PEP8 wants this:
log.warn("The build %s is a part of two different updates:
'%s'"
" and '%s'. That's probably a bug in
Bodhi." % (build,
failures[build]['title'],
build2update[build]['title'])
)
In a longer line of text, having the closing bracket really matching the opening one (the
first example) is much easier on the eyes.
==== E122 =====
libtaskotron/runner.py:127:17: E122 continuation line missing indentation or outdented
raise TaskotronYamlError("Input yaml should contain correct
'input'"
"section (a mapping). Yours was: %s" %
type(
self.taskdata['input']))
I don't really understand the purpose of this error. If I move 'type(' to the
third line, it goes away.
==== E265 ====
libtaskotron/directives/bodhi_comment_directive.py:422:13: E265 block comment should start
with '# '
#TODO: replace url with proper URL
Me and Josef are used to avoid the space in these cases. We can re-learn, of course. But
it's very common to write it this way.
==== E111, E113 ====
libtaskotron/config_defaults.py:53:35: E111 indentation is not a multiple of four
libtaskotron/config_defaults.py:53:35: E113 unexpected indentation
bodhi_posting_comments_span = 4320 #:
# 3 days (3*24*60 = 4320)
This is a nice example how automated style checks impede visual coding. In this case, it
makes absolute sense to have the comment written in this way. However, if we suppress E111
and E113 globally, we might miss some potential problems elsewhere. I'm not sure if we
can suppress certain warnings only in a selected block of code, but if we check for PEP8
automatically, we should find out. There will be more use cases like this one.
Sooo, I'm not exactly a big fan of automated PEP8 checking (I had to disable it in
Spyder, because it was driving me mad), but it might be somewhat useful, if we configure
some exceptions to the general rules.