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 <f...@redhat.com> >>> Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> >>> Signed-off-by: Peter Xu <pet...@redhat.com> >>> --- >>> qom/object.c | 9 +++------ >>> 1 file changed, 3 insertions(+), 6 deletions(-) >> >> Reviewed-by: Eric Blake <ebl...@redhat.com> >> >> 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