qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 03/22] qapi/source: Remove line number from QAPISourceInfo in


From: Markus Armbruster
Subject: Re: [PATCH 03/22] qapi/source: Remove line number from QAPISourceInfo initializer
Date: Tue, 27 Apr 2021 08:07:24 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)

John Snow <jsnow@redhat.com> writes:

> On 4/24/21 2:38 AM, Markus Armbruster wrote:
>> Mixing f-string and % interpolation.  I doubt we'd write it this way
>> from scratch.  I recommend to either stick to % for now (leave
>> conversion to f-strings for later), or conver the column formatting,
>> too, even though it's not related to the patch's purpose.
>
> True. Two thoughts:
>
> 1. I don't like using % formatting because it behaves differently from
> .format() and f-strings. My overwhelming desire is to never use it for 
> this reason.
>
> Example: {foo} will call foo's __format__ method, whereas "%s" % foo
> will simply add str(foo). They are not always the same, not even for 
> built-in Python objects.

I only care for readability, which profits from local consistency.
Maybe I'll sing a different tune once I got actually bitten by the
difference between interpolation and f-strings.

> 2. Cleaning up the formatting here without cleaning it up everywhere
> is a great way to get the patch NACKed. You have in the past been
> fairly reluctant to "While we're here" cleanups, so I am trying to cut
> back on them.

Yes, I've been pushing back on such cleanups.  But it's really a
case-by-case issue.  When a patch fits on a page, squashing in a bit of
losely related cleanup is usually fine.  When it's long, keep it focused
on a single purpose.

> This is why my habit for f-strings keeps trickling in: whenever I have
> to rewrite any interpolation, I reach for the one that behaves most 
> idiomatically for Python 3. I am trying to balance that against churn
> that's not in the stated goals of the patch.
>
> In this case: I'll clean the rest of the method to match; and add a
> note to the commit message that explains why.

Okay.

>                                               I will get around to
> removing all of the f-strings,

The opposite, I presume.

>                                but I want to hit the clean linter
> baseline first to help guide the testing for such a series. I regret
> the awkward transitional period.

I'd leave converting interpolation to f-strings for later.

I can tolerate early, partial conversion, since I trust complete
conversion will happen, and as long as the resulting local inconsistency
isn't too grating.  Subjective, I know.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]