Re: [Qemu-devel] [RFC v6 04/27] qobject: let object_property_get_str() use new API

[Eric Blake]

On 01/10/2018 01:57 AM, Peter Xu wrote:
> On Tue, Jan 09, 2018 at 04:53:40PM -0600, Eric Blake wrote:
>> On 12/19/2017 02:45 AM, Peter Xu wrote:
>>> We can simplify object_property_get_str() using the new
>>> qobject_get_try_str().
>>>
>>> Reviewed-by: Fam Zheng <address@hidden>
>>> Reviewed-by: Stefan Hajnoczi <address@hidden>
>>> Signed-off-by: Peter Xu <address@hidden>
>>> ---
>>>  qom/object.c | 9 +++------
>>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> Reviewed-by: Eric Blake <address@hidden>
>>
>> I'm not opposed to your patch split (particularly since it makes
>> backports easier if it just needs the new function and then your later
>> uses of the new function, without touching existing uses); but I might
>> have merged this with the previous patch so that the new API has a
>> client right away, proving why the new API is worthwhile as part of its
>> introduction.
> 
> Sure, I'll follow the rule next time.
> 
> (I can simply squash it but I got a few r-bs for separate patches
>  already, so I'll keep them separated for now)

Keeping it separate is fine, especially since you have r-b.

Another factor in my personal workflow - if I'm going to be converting
lots of users, but the users span over multiple areas of the code, then
introducing the helper in one patch, and then a series of conversions to
start using it, makes sense.  But if I'm converting just one existing
user, that's a refactoring, and doing it all at once is easier (prior to
adding the new code that becomes the second user).  I guess the moral of
the story is that patch splitting is an art form, and there's more than
one way to do things that still end up nice and reviewable.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature