[Buildbot-devel] Follow up on autopep8

[Pierre Tardy]

>
>
> If main Buildbot developers feels comfortable with following PEP8 (or part
> of advices in it) and/or using git hook suggested by Pierre and/or running
> autopep8, I think we should accept as much as we can from PEP8.
>
> About fixing PEP8 issues with autopep8: I would prefer to fix issues one
> by one, but in short period of time (e.g. one or two weeks) and close to
> moment when another merge of master to nine planned.  With one by one fix
> it will be easier to see how each autopep8 issue fix looks like, what it
> breaks, what it makes better and what worse.  It's impossible to analyze
> one large fix of all issues: I bet you won't be able to find at least one
> change for each fixed by autopep8 issue just by looking in 19kloc patch.
>
> I suggest following steps:
> 1) Take simple not fixed PEP8 issue, let it be W123.
> 2) Run autopep8 to fix it:
>
> find master/ slave/ www/ -iname '*.py' -exec autopep8 --aggressive \
>     --verbose --in-place --max-line-length=999 --select=W123 \
>     '{}' \;
>
> 3) Commit and push branch on Github --- wait till Travis will end running
> tests.
> 4) If any tests broken: fix them and go to 3).
> 5) Analyze changes and discuss them with other Buildbot developers --- is
> changes ok, makes sence or poinless and makes code worse? Accept changes or
> decide to ignore W123.
> 6) Go to 1).
>

So I disagree with this proposal.
- This will take too much time and review efforts. Autopep8 is quite slow.
It takes 1.5 hour to run on our entire codebase, so you dont want to run it
too much time
- In the end you will still have the 20k loc changes, and people will still
have pain to rebase.

My proposal is to make the biggest and more painful step in a big patch. We
have a very good unit test suite, so I used that to make sure I select
changes in autopep8 that does not break the tests.

For example W6 fixes and --aggressive options are doing changes provided by
lib2to3 that are resulting in code not supported by python 2.5. So I
disabled them in the automated fixes. We still have the option to run them
separatly with https://pypi.python.org/pypi/modernize.

I did a script that automates merge conflicts resolution for people willing
to contribute code based from the pre-autopep8 codebase. I run it on the
worst case we probably have now which is nine branch, and it can now run
without human intervention.
I volunteer of course to help people run it, or even handle the merges
myself if necessary.

Here are my 3 branches:
pep8: code before the big autopep8 run
https://github.com/buildbot/buildbot/pull/935

pep8fix: pep8 branch + big 1.5hours autopep8 run (with 2to3 fixes), and
then various commits to fix the issues generated by lib2to3.
https://github.com/buildbot/buildbot/pull/936

ninepep8fix: the automated branch merge generated by merge_and_pep8.sh on
the nine branch
https://github.com/buildbot/buildbot/pull/943

Regards,
Pierre
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://buildbot.net/pipermail/devel/attachments/20131102/5956ea85/attachment.html>