[Buildbot-devel] Using pep8 on Buildbot source code

Mon Oct 21 20:19:40 UTC 2013

We have been using stricter pep8 at work for a bit of time.

It really helped us to have a more coherent and fluid code base. Even if
some of them are arguable aestetic matters, this is still a clear and
automated decision on what to do.

Its is very easy to setup most editors so that the pep8 errors are shown
directly in the editor. Our team has emacs, aptana, and sublimetext
developers, so I can contribute some docs on how to setup those.
We also have a git post commit hooks that run autopep8 on the files
modified by the commit, and which reorders the import in alphabetical order.
This is to ease automation of backporting of patches between our
stabilization branches.
I can contribute it as well.

Actually going back working on buildbot upstream looks ugly for me because
a lot of those pep8 error are there, which puts red flags on lots of lines.

I often asked myself wether I should suggest to run autopep8, and our
import sorting on the whole code of buildbot.

basically this means:
merge master into nine -> resolve merge conflicts once.
checkout master
autopep8
checkout nine
autopep8
merge master
resolve all the conflicts with "--ours" option.

The problem, is that then it may be a bit painful for people who have not
yet contributed their patches. However the same method can be applied (i.e.
merge the last patch from mainline, apply autopep8, merge the first patch
after autopep8, with --ours resolution, and remerge everything. ) This
could even be automated in a contrib script.

Pierre

On Mon, Oct 21, 2013 at 6: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?
>
> > 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
>
>
> ------------------------------------------------------------------------------
> October Webinars: Code for Performance
> Free Intel webinars can help you accelerate application performance.
> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most
> from
> the latest Intel processors and coprocessors. See abstracts and register >
> http://pubads.g.doubleclick.net/gampad/clk?id=60135031&iu=/4140/ostg.clktrk
> _______________________________________________
> Buildbot-devel mailing list
> Buildbot-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/buildbot-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://buildbot.net/pipermail/devel/attachments/20131021/1e4c6b57/attachment.html>