From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43776) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cCB5p-0001ag-Db for qemu-devel@nongnu.org; Wed, 30 Nov 2016 15:03:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cCB5l-0001RX-G7 for qemu-devel@nongnu.org; Wed, 30 Nov 2016 15:03:01 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47450) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cCB5l-0001R3-9Y for qemu-devel@nongnu.org; Wed, 30 Nov 2016 15:02:57 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AABD18F27A for ; Wed, 30 Nov 2016 20:02:55 +0000 (UTC) References: <9151aa4b-44ed-ee0f-4dac-581931b3e899@redhat.com> <2398cef0-35e9-0877-434f-506037c72c45@redhat.com> From: Laszlo Ersek Message-ID: Date: Wed, 30 Nov 2016 21:02:53 +0100 MIME-Version: 1.0 In-Reply-To: <2398cef0-35e9-0877-434f-506037c72c45@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] a suggestion to place *.c hunks last in patches List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu devel list 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 >> Output the patch in the order specified in the , >> 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