[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v8 07/11] iotests: limit line length to 79 chars
From: |
Max Reitz |
Subject: |
Re: [PATCH v8 07/11] iotests: limit line length to 79 chars |
Date: |
Tue, 24 Mar 2020 16:12:22 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 |
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 <address@hidden>
>> ---
>> 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
signature.asc
Description: OpenPGP digital signature
- Re: [PATCH v8 06/11] iotests: drop Python 3.4 compatibility code, (continued)
[PATCH v8 09/11] iotest 258: use script_main, John Snow, 2020/03/16
[PATCH v8 10/11] iotests: Mark verify functions as private, John Snow, 2020/03/16
[PATCH v8 11/11] iotests: use python logging for iotests.log(), John Snow, 2020/03/16
[PATCH v8 08/11] iotests: add script_initialize, John Snow, 2020/03/16