On 05/14/2015 10:09 AM, Markus Armbruster wrote: > Eric Blake 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 > > 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