From: Laszlo Ersek <lersek@redhat.com>
To: Eric Blake <eblake@redhat.com>, qemu devel list <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] a suggestion to place *.c hunks last in patches
Date: Wed, 30 Nov 2016 21:02:53 +0100 [thread overview]
Message-ID: <aeef4979-7aad-b08a-a376-fb6c059dee13@redhat.com> (raw)
In-Reply-To: <2398cef0-35e9-0877-434f-506037c72c45@redhat.com>
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
prev parent reply other threads:[~2016-11-30 20:03 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-30 10:08 [Qemu-devel] a suggestion to place *.c hunks last in patches Laszlo Ersek
2016-11-30 10:55 ` Gerd Hoffmann
2016-11-30 12:03 ` Laszlo Ersek
2016-11-30 12:27 ` Fam Zheng
2016-12-02 10:15 ` Laszlo Ersek
2016-11-30 12:29 ` Gerd Hoffmann
2016-11-30 15:36 ` Laszlo Ersek
2016-11-30 20:48 ` John Snow
2016-11-30 21:54 ` Laszlo Ersek
2016-12-02 10:23 ` Stefan Hajnoczi
2016-11-30 15:08 ` Michael S. Tsirkin
2016-11-30 15:26 ` Laszlo Ersek
2016-11-30 18:31 ` Michael S. Tsirkin
2016-11-30 15:35 ` Stefan Hajnoczi
2016-11-30 15:41 ` Eric Blake
2016-11-30 20:02 ` Laszlo Ersek [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aeef4979-7aad-b08a-a376-fb6c059dee13@redhat.com \
--to=lersek@redhat.com \
--cc=eblake@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).