Add PEP8 check to Travis-CI #3273
Conversation
| @@ -38,5 +38,5 @@ commands=python {toxinidir}/runtests.py -n -m full {posargs:} | |||
| [pep8] | |||
| max_line_length=79 | |||
| statistics = True | |||
| ignore = E121,E122,E123,E125,E126,E127,E128,E226,E231,E501,E712 | |||
| ignore = E121,E122,E123,E125,E126,E127,E128,E226,E231,E501,E712,W291,W293,W391 | |||
There was a problem hiding this comment.
Why ignore all the whitespace related warnings? That passed until recently, so I suggest to keep that as is.
There was a problem hiding this comment.
I don't find that trailing whitespace harms readability, so I don't see the point in checking for it. (The check does not pass any more.)
There was a problem hiding this comment.
Agreed, it doesn't harm readability. However some editors (like Vim, which I use) automatically strip trailing whitespace, so you still get commits with diffs for lines that have nothing to do with the actual code changes in those commits. If you prefer not checking for it that's fine with me, just want to point out that there is a good reason for the check.
There was a problem hiding this comment.
Yes. However, I suspect that enforcing that is more hassle than useful.
It's probably embarrassing enough to tell people to fix their visible whitespace :)
There was a problem hiding this comment.
But I have vim set up to show trailing whitespace. It's always visible and ugly ;)
|
Ironically, has a merge conflict:) |
|
+1 for enabling this. Will be a pain for a while, but it teaches people to set up their editor correctly. |
|
Rebased |
|
Time to merge this? |
|
Rebased once again. I don't see blockers for merging. |
|
OK in it goes. Thanks @pv. |
|
Any reason not to do pep8 and pyflakes on the same worker? |
|
Not really |
Not sure if this is a good idea, but it should eliminate whitespace fixing commits that can cause merge conflicts.