Re: [Qemu-devel] a suggestion to place *.c hunks last in patches

[Laszlo Ersek]

On 11/30/16 16:41, Eric Blake wrote:
> On 11/30/2016 04:08 AM, Laszlo Ersek wrote:
>> Recent git releases support the diff.orderFile permanent setting. (In
>> older releases, the -O option had to be specified on the command line,
>> or in aliases, for the same effect, which was quite inconvenient.) From
>> git-diff(1):
>>
>>        -O<orderfile>
>>            Output the patch in the order specified in the <orderfile>,
>>            which has one shell glob pattern per line. This overrides
>>            the diff.orderFile configuration variable (see git-
>>            config(1)). To cancel diff.orderFile, use -O/dev/null.
>>
>> In my experience, an order file such as:
>>
>> configure
>> *Makefile*
>> *.json
>> *.txt
>> *.h
>> *.c
>>
>> that is, a priority order that goes from
>> descriptive/declarative/abstract to imperative/specific works wonders
>> for reviewing.
> 
> I've been doing this for some time now, although I sometimes tweak the
> order on a per-series basis to highlight what I think is most
> interesting in that series.  My current order file contents are:
> 
> qapi-schema*.json
> qapi/*.json
> include/qapi/visitor.h
> include/qapi/visitor-impl.h
> scripts/qapi.py
> scripts/*.py
> *.h
> qapi/qapi-visit-core.c
> *.c
> 
> stemming from a patch series that touched visitor interfaces.
> 

It should also be noted -- I forgot before, sorry -- that the diff order
affects any and all diffstats as well. This is usually welcome for
individual patches (the diffstat should reflect the order of hunks in
the patch), but quite unhelpful for the cumulative diffstat across a
series (in the blurb).

I cope with this by selecting the cumulative diffstat lines in my
editor, when editing the blurb, and filtering that block through "sort".
Most of the time it works fine, without needing further touch-ups.

When the series copies or moves files (with the appropriate copy/move
detection enabled), then the cumulative diffstat (too) contains those
funky { and } characters. They don't play too nice with "sort", hence
manual reordering could be necessary in this case, for a review
experience de luxe.

Laszlo