Re: [PATCH v8 07/11] iotests: limit line length to 79 chars

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2329 bytes --]

On 24.03.20 16:02, Max Reitz wrote:
> On 17.03.20 01:41, John Snow wrote:
>> 79 is the PEP8 recommendation. This recommendation works well for
>> reading patch diffs in TUI email clients.
> 
> Also for my very GUI-y diff program (kompare).
> 
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  tests/qemu-iotests/iotests.py | 64 +++++++++++++++++++++++------------
>>  tests/qemu-iotests/pylintrc   |  6 +++-
>>  2 files changed, 47 insertions(+), 23 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 3d90fb157d..75fd697d77 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
> 
> [...]
> 
>> @@ -529,11 +539,13 @@ def pause_drive(self, drive, event=None):
>>              self.pause_drive(drive, "write_aio")
>>              return
>>          self.qmp('human-monitor-command',
>> -                 command_line='qemu-io %s "break %s bp_%s"' % (drive, event, drive))
>> +                 command_line='qemu-io %s "break %s bp_%s"'
>> +                 % (drive, event, drive))
> 
> Can we put this value in a variable instead?  I don’t like the %
> aligning with the parameter name instead of the string value.  (I also
> don’t like how there are no spaces around the assignment =, but around
> the %, even though the % binds more strongly.)
> 
>>  
>>      def resume_drive(self, drive):
>>          self.qmp('human-monitor-command',
>> -                 command_line='qemu-io %s "remove_break bp_%s"' % (drive, drive))
>> +                 command_line='qemu-io %s "remove_break bp_%s"'
>> +                 % (drive, drive))
>>  
>>      def hmp_qemu_io(self, drive, cmd):
>>          '''Write to a given drive using an HMP command'''
>> @@ -793,16 +805,18 @@ def dictpath(self, d, path):
>>                  idx = int(idx)
>>  
>>              if not isinstance(d, dict) or component not in d:
>> -                self.fail('failed path traversal for "%s" in "%s"' % (path, str(d)))
>> +                self.fail(f'failed path traversal for "{path}" in "{d}"')
> 
> Do we require 3.6 so that f-strings are guaranteed to work?  (I thought
> we didn’t.  I’d be happy to use them.)

Oh.  Of course we do.  It says so in this file that this whole series is
about.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]