qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v4 16/16] qapi: Prefer '"str" + var' over '"str%


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v4 16/16] qapi: Prefer '"str" + var' over '"str%s" % var'
Date: Thu, 14 May 2015 10:25:40 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0

On 05/14/2015 10:09 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> Use of python's % operator to format strings is fine if there are
>> multiple strings or if there is integer formatting going on, but
>> when it is just abused for string concatenation, it looks nicer
>> to just use the + operator.  This is particularly true when the
>> value being substituted is at the front of the format string,
>> rather than the tail.
> 
> I quite agree in cases such as 
> 
> -        discriminator_type_name = '%sKind' % (name)
> +        discriminator_type_name = name + 'Kind'

I could always split this into the obvious cases vs. the questionable
ones, if that helps.

> 
> I have doubts in cases such as
> 
> -    return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_name(name)
> +    return "qmp_marshal_output_" + c_name(name) + "(retval, ret, 
> &local_err);"
> 
> I find the old code makes it easier to grasp the result.  Admittedly
> subjective.

Yeah, that's a judgment call.

> 
>> Update an error message (and testsuite coverage) while at it, since
>> concatenating a non-string object does not always produce legible
>> results.
> 
> The new expected test output shows improvement.

Also might be worth splitting into its own patch, rather than buried in
the noise of cleanups.

> 
>> Signed-off-by: Eric Blake <address@hidden>
> 
> I'll take 01-15 now, and have a second look at this one later, okay?

Yeah, I kind of figured that might happen. Works for me :)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

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