[Buildbot-devel] Using pep8 on Buildbot source code

[Vladimir Rutsky]

On Mon, Oct 21, 2013 at 8:53 PM, Dustin J. Mitchell <dustin at v.igoro.us> wrote:
> My worry with a lot of these is that they can be burdensome for
> contributors, without adding a lot of value.  So:
>
>> 145     E101 indentation contains mixed spaces and tabs
>> 538     E111 indentation is not a multiple of four
>> 891     E203 whitespace before ':'
>> 13      E211 whitespace before '('
>> 932     E251 unexpected spaces around keyword / parameter equals
>> 35      E502 the backslash is redundant between brackets
>> 78      E701 multiple statements on one line (colon)
>> 8       E703 statement ends with a semicolon
>> 79      E711 comparison to None should be 'if cond is None:'
>> 23      E712 comparison to False should be 'if cond is False:' or 'if
>> not cond:'
>> 15      E721 do not compare types, use 'isinstance()'
>> 117     W191 indentation contains tabs
>> 316     W291 trailing whitespace
>> 3       W292 no newline at end of file
>> 421     W293 blank line contains whitespace
>> 95      W391 blank line at end of file
>> 47      W601 .has_key() is deprecated, use 'in'
>> 16      W602 deprecated form of raising exception
>> 11      W604 backticks are deprecated, use 'repr()'
>
> These are important.
>
>> 272     E121 continuation line indentation is not a multiple of four
>> 59      E122 continuation line missing indentation or outdented
>> 87      E124 closing bracket does not match visual indentation
>> 35      E125 continuation line does not distinguish itself from next logical line
>> 1314    E126 continuation line over-indented for hanging indent
>> 522     E127 continuation line over-indented for visual indent
>> 3487    E128 continuation line under-indented for visual indent
>
> IMHO, continuation lines are an aesthetic matter, and not something we
> should be enforcing.
>
>> 1010    E201 whitespace after '['
>> 933     E202 whitespace before ']'
>
> I tend to do this around list comprehensions.  I could try to stop, I suppose :)
>
>> 36      E221 multiple spaces before operator
>> 12      E222 multiple spaces after operator
>> 182     E225 missing whitespace around operator
>> 10      E227 missing whitespace around bitwise or shift operator
>> 30      E228 missing whitespace around modulo operator
>> 820     E231 missing whitespace after ','
>> 10      E271 multiple spaces after keyword
>> 12      E272 multiple spaces before keyword
>
> I sometimes use these (esp E225) to make expressions fit on lines.
>
>> 586     E261 at least two spaces before inline comment
>
> I think this one is particularly silly..
>
>> 17      E262 inline comment should start with '# '
>
> I don't understand this one.  Are there other options for starting
> comments in Python?

It should be read literally: comment should start with hash sign,
followed by space ('# ').
It applies to code such as

...
    import setuptools #@UnusedImport
...
            slavedest='dir', ## but that's a directory!
...
            if source.patch_info: #Add patch author to blamelist

IMO it is a good style to put space between '#' and comment text.

>
>> 1138    E301 expected 1 blank line, found 0
>> 1045    E302 expected 2 blank lines, found 1
>> 247     E303 too many blank lines (2)
>
> I think it would probably do us well to adhere to this pattern, even
> if it is a *huge* change.
>
>> 39      E401 multiple imports on one line
>
> Fixing this would make pyflakes a little more reliable.
>
>> 2545    E501 line too long (80 > 79 characters)
>
> Yuck.  I try to check this on commits, and reject ridiculously long
> lines, but 80 characters is pretty short for some of the method names
> and dotted paths in Buildbot.
>
> This will be tricky to do on the master and nine branches, but you did
> a wonderful job with pylint, so I'm not worried.
>
> Dustin