GNOME Bugzilla – Bug 779752
[patch attached] Add a pyflakes check
Last modified: 2017-11-17 14:13:09 UTC
Created attachment 347484 [details] [review] Add a pyflakes check to the Python backend The attached patch adds a pyflakes check to GCA. I've hidden it behind an option in the same way as the pylint one.
Elad, can you review this?
Review of attachment 347484 [details] [review]: Thank you for this patch, seems that pyflakes covers cases that pylint and pep8 do not, so it's a nice addition to the python backend. Overall it looks looks (almost) good. message not being defined is the biggest issue here, the other two are minor. ::: backends/python/__init__.py @@ +92,3 @@ + loc = types.SourceLocation(line=0, column=0) + severity = types.Diagnostic.Severity.ERROR + text = message.message % message.message_args message is not defined here, maybe you meant msg? @@ +110,3 @@ + loc = types.SourceLocation(line=message.lineno, + column=int(message.col) + 1) + severity = types.Diagnostic.Severity.INFO Shouldn't the severity of this be WARNING instead of INFO? @@ +161,3 @@ doc.diagnostics.append(diag) + if HAS_PYFLAKES and options.get("pyflakes", False): I'm not sure it's a good idea to have pyflakes disabled by default, or to have an option to disable it at all. Why is this needed? pyflakes doesn't execute files, just parses them (or so it claims), so I don't think there are any obvious security implications of running it on untrusted code. I suggest to change this condition to simply if HAS_PYFLAKES:
Review of attachment 347484 [details] [review]: Committed as 4176c4f4eaeab7f8f64842e4d879871720a4b39b (with minor modifications)
https://git.gnome.org/browse/gnome-code-assistance/commit/?id=4176c4f4eaeab7f8f64842e4d879871720a4b39b
Hi! Sorry I've been trying to find time to update the patch. Thank you for tweaking and merging!!