Re: [PATCH 1/2] pylint: fix errors and warnings from qemu-tests test 297

[Kevin Wolf]

Am 07.10.2021 um 09:51 hat Emanuele Giuseppe Esposito geschrieben:
> 
> 
> On 06/10/2021 18:46, Kevin Wolf wrote:
> > Am 06.10.2021 um 15:00 hat Emanuele Giuseppe Esposito geschrieben:
> > > Test 297 in qemu-iotests folder currently fails: pylint has
> > > learned new things to check, or we simply missed them.
> > > 
> > > All fixes in this patch are related to additional spaces used
> > > or wrong indentation.
> > > 
> > > No functional change intended.
> > > 
> > > Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> > 
> > > @@ -87,13 +87,14 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
> > >           iotests.qemu_img('create', '-f', iotests.imgfmt, 
> > > self.overlay_img,
> > >                            '1G')
> > > -        result = self.vm.qmp('blockdev-add', **{
> > > +        result = self.vm.qmp('blockdev-add',
> > > +                             **{
> > >                                    'node-name': 'overlay',
> > >                                    'driver': iotests.imgfmt,
> > >                                    'file': {
> > >                                        'driver': 'file',
> > >                                        'filename': self.overlay_img
> > > -                                 }
> > > +                                     }
> > >                                })
> > >           self.assert_qmp(result, 'return', {})
> > 
> > Am I the only one to think that the new indentation for the closing
> > brace there is horrible? PEP-8 explictly allows things like:
> > 
> >      my_list = [
> >          1, 2, 3,
> >          4, 5, 6,
> >      ]
> > 
> > Some of the other changes in this patch should be made, but at least if
> > these are behind different switches, I would consider just disabling the
> > one that complains about nicely formatted dicts.
> 
> The error is "C0330: Wrong hanging indentation"
> so it is not about dicts. I guess we can disable the error, but the problem
> is that we will disable it for the whole file, which doesn't seem right.

Actually, I would disable it globally in pylintrc because building
dictionaries for JSON is something that we do a lot.

But then I'm surprised that this is the only instance that actually
fails. I wonder what the difference is.

For example, 129 doesn't seem to be skipped and has this code:

    result = self.vm.qmp('blockdev-add', **{
                             'node-name': 'overlay',
                             'driver': iotests.imgfmt,
                             'file': {
                                 'driver': 'file',
                                 'filename': self.overlay_img
                             }
                         })

Yet you don't report a pylint error for this file.

Oooh... I think I do see a difference: The final line is indented by one
space more in the case that fails for you. It should be vertically
aligned with the "'" in the first line, but it is actually aligned with
the "b" of "blockdev-add".

Does removing one space of indentation in the last line fix the report?

Kevin