Re: [PATCH] tests: add a "check-flake8" test for validating python code style

[Daniel P . Berrangé]

On Thu, Apr 30, 2020 at 12:29:28PM +0200, Paolo Bonzini wrote:
> On 29/04/20 17:36, Daniel P. Berrangé wrote:
> > The flake8 program is a standard tool used by Python projects for
> > validating many commonly recommended style rules. It would be desirable
> > for QEMU to come into alignment with normal Python coding style best
> > practices.
> > 
> > QEMU currently violates a huge number of the style rules, so we can't
> > blindly turn it on. Instead this patch introduces use of flake8 with
> > a huge ignore list to turn off everything that is currently violated.
> > 
> > The following descriptions are mostly taken from:
> > 
> >   https://www.flake8rules.com/
> 
> I suggest instead using "black" and just reformat everything in a huge
> patch; that's what we're using for Patchew.

Personally I'm a fan of automated code formatters that can enforce a
specific style, so no objection from me.

> It's not perfect, and especially one needs to get used to the double
> quotes instead of single quotes for strings.  However overall it is
> pretty good, and I've never seen it do something clearly "wrong".

I've not looked at black in any detail, but if its focused on code
style, then there might be a few bits from flake8 that are still
useful, such as checking for unused imports, or unresolved variable
names. Then again, perhaps Jon's pylint stuff will already catch
that.

> > On its own this patch doesn't really do much of use except try to stop the
> > situation getting worse. To be valuable some motivated contributor(s)
> > would need to go through fixing the code, and re-enabling each excluded
> > warning category one at a time.
> > 
> > I'm mostly proposing this patch as a starting point for discussion, to
> > see if anyone is indeed motivated to take on the code cleanup challenge,
> > and feed the fixes in through the various maintainers trees.
> 
> If we go with "black" I suggest just doing a big patch for everything,
> since it's easy for maintainers to rebase by running black at each step.
>  Overall our usage of Python is small enough that it should not be a big
> deal.



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|