Re: [PATCH v3 11/10] iotests: add flake8 linter

[Vladimir Sementsov-Ogievskiy]

15.01.2021 15:03, Max Reitz wrote:
> On 15.01.21 12:53, Vladimir Sementsov-Ogievskiy wrote:
> > pylint is good, but doesn't cover the PEP8. Let's add flake8, to be
> > sure that our code sutisfy PEP8. Add new linter and fix some code style
> > in checked files.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > ---
> >
> > Hi!
> >
> > Here is my small addition to Max's series, hope you like it!
> >
> > Note, that this is not the first occurrence of 'flake8' in Qemu:
> >
> >      # git grep flake8
> >      python/qemu/.flake8:[flake8]
> >      scripts/qapi/.flake8:[flake8]
> >      scripts/qapi/.flake8:extend-ignore = E722  # Prefer pylint's bare-except 
> > checks to flake8's
> >
> >
> >   tests/qemu-iotests/129        |  6 ++-
> >   tests/qemu-iotests/254        |  2 +-
> >   tests/qemu-iotests/297        | 21 ++++++---
> >   tests/qemu-iotests/297.out    |  1 +
> >   tests/qemu-iotests/300        |  4 +-
> >   tests/qemu-iotests/iotests.py | 88 +++++++++++++++++++++++++++++++++--
> >   6 files changed, 106 insertions(+), 16 deletions(-)
>
> Looks reasonable to me, but perhaps it should just be a dedicated series.  I 
> think there’s enough in here to justify that.

Not a problem, I can resend :)

> > diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
> > index 201d9e0a0b..28e6666c1d 100755
> > --- a/tests/qemu-iotests/129
> > +++ b/tests/qemu-iotests/129
> > @@ -21,6 +21,7 @@
> >   import os
> >   import iotests
> > +
> >   class TestStopWithBlockJob(iotests.QMPTestCase):
> >       test_img = os.path.join(iotests.test_dir, 'test.img')
> >       target_img = os.path.join(iotests.test_dir, 'target.img')
> > @@ -39,8 +40,8 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
> >           source_drive = 'driver=throttle,' \
> >                          'node-name=source,' \
> >                          'throttle-group=tg0,' \
> > -                      f'file.driver={iotests.imgfmt},' \
> > -                      f'file.file.filename={self.test_img}'
> > +                       f'file.driver={iotests.imgfmt},' \
> > +                       f'file.file.filename={self.test_img}'
>
> Interesting, when indenting this, I was wondering whether pylint would 
> complain.  I was so glad it didn’t.  I really don’t like PEP8.
>
> (Though I understand that style guides like PEP8 are there specifically so when 
> someone like me goes “but I like this style better :(”, everyone else can say 
> “but you’re objectively wrong”.  So me hating it kind of is its point, I guess.)

When I noted how you indent 'string' with f'string', I thought "o, interesting 
idea".. I just very like the idea of standardized code-style for the language, so I 
just used to follow PEP8 since first time I learned about it.

--
Best regards,
Vladimir