On Wed, Jan 10, 2018 at 06:59:13AM -0600, Eric Blake wrote: > 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.
Agreed. I started to learn how to split patches since when I started to work on QEMU, and obvious now I'm still learning. :-) Thanks Eric. -- Peter Xu
