On 24.03.20 18:09, John Snow wrote: > > > On 3/24/20 11:12 AM, Max Reitz wrote: >> 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 <js...@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.) >>> >>>> > > I think I had another patch somewhere that added an HMP helper that > fixes this issue for this particular instance. > > I can send that separately as a follow-up. I think otherwise, > unfortunately, "we" "decided" that this indent style is "best". > > So I think that this patch is "correct".
Perhaps it’s the best if we assume that we had to do it this way, but can’t we just do a qemu_io_cmd = f'qemu-io {drive} "break {event} bp_{drive}"' self.qmp('human-monitor-command', command_line=qemu_io_cmd) ? (Or maybe an f-string inline. I was thinking of a separate variable because I wasn’t aware that f-strings would be an option until I got to later hunks of this patch...) > (All of the other options for indent styles seemed to be worse visually, > or actively go against PEP8. While PEP8 is not a bible, every conscious > choice to disregard it generally means you will be fighting a CQA tool > at some other point in time. I have therefore adopted a "When in Rome" > policy to reduce friction wherever possible with pylint, flake8, mypy, > pycharm, and so on.) > > > ((I would prefer we begin to deprecate uses of % and begin using > .format() and f-strings wherever possible to help alleviate this kind of > syntax, too -- but I must insist that's for another series.)) Hm. But you do change something to f-strings below, why not here? Max
signature.asc
Description: OpenPGP digital signature