GNOME Bugzilla – Bug 725621
[patch attached] Check for PEP8 coding-style errors in Python files
Last modified: 2014-03-04 19:03:06 UTC
Created attachment 270848 [details] [review] [backends/python] Check for PEP8 coding-style errors Patch attached :)
Created attachment 270849 [details] [review] [backends/python] Check for PEP8 coding-style errors Minor improvement over the previous version: don't die if PEP8 is not installed.
This is nice! Two comments though: 1. PEP8 is not an actual requirement for a correct program. I wonder if we should have a way to disable certain parts of the diagnostics (i.e. like -Wno-pep8, but nicer of course). This doesn't really concern this patch specifically, but we should maybe think about implementing this in general. 2. It would be nice not to shell out if possible. I had a quick look at the pep8 module on pypi and it looks like it should be relatively easy to import it and use it directly instead of using Popen. You would also not need to parse the output afterwards but can just use its API.
I agree about #1 Regarding #2: it's not a proper API. It outputs to stdout, and if you wrap that with StringIO it blocks forever. I spent an hour and a half last night trying to make that work, and couldn't. Someone said I could change the Reporter class to make it not use print() but it's not documented well enough, the Reporter class structure is unclear and there are calls to print() everywhere even outside the Reporter class Using Popen is easier and also means much less lines of code. It's ugly, but it works.
Ok, lets file #1 under another bug. Please go ahead and push your patch, nice work!
Review of attachment 270849 [details] [review]: Pushed