[libvirt] [PATCH v4 0/4] qemu_process: Glib sponsored cleanup

[Michal Prívozník]

On 12/20/19 7:57 PM, Cole Robinson wrote:
> On 12/19/19 4:09 PM, Daniel Henrique Barboza wrote:
>> This series got buried a few months ago. Let's go onward unto
>> the 20s with no one left behind.
>>
>> changes from version 3 [1]:
>> - rebased to master at commit 330b556829
>> - removed former patch 4. The 'g_strdup_printf' change was
>> made along the road
>> - new patch 4: I am sending this one in separate to patch 3
>> because this unneeded label was already in the code, being
>> unrelated to the changes of this series. The maintainer is
>> welcome to squash it into patch 3.
>>
>> [1] https://www.redhat.com/archives/libvir-list/2019-October/msg01080.html
>>
>> Daniel Henrique Barboza (4):
>>   qemu_process.c: use g_autofree
>>   qemu_process.c: use g_autoptr()
>>   qemu_process.c: remove cleanup labels after g_auto*() changes
>>   qemu_process.c: remove 'cleanup' label from
>>     qemuProcessCreatePretendCmd()
>>
>>  src/qemu/qemu_process.c | 429 +++++++++++++++-------------------------
>>  1 file changed, 157 insertions(+), 272 deletions(-)
>>
> 
> Patch 3 and 4 look fine, some comments on the first two.
> 
> I can't really decide what is the best way to approach cleanups like
> this. Should patches be split by function, and do all cleanups in one
> pass, or do one type of cleanup, but over a larger surface per file?
> Like you have done here.
> 
> The first method is more tedious, but it's easier for reviewers, and
> good patches can go in first while patches with issues can be kicked out
> for v2. But it could be thousands of patches judging by the 3000+
> cleanup labels we have in the code base, which sounds extreme.
> 
> I think in general you will find the list more receptive to series of
> small per function patches though. Optimizing for ease of review will
> always give things a better chance of getting committed in my
> experience. This is just recommendation for future series though, I'll
> review this one as is

I think the sooner we get this over with the better (i.e. less rebase
conflicts). It's like tearing off a patch (bandage?) - it hurts less if
you do it all at once.

Having said that, it still makes sense to honour our rules and have one
semantic chnage per commit.

Michal