From: Jeff King <peff@peff.net>
To: Michael J Gruber <git@drmicha.warpmail.net>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
Jonathan Nieder <jrnieder@gmail.com>
Subject: Re: [PATCHv3 0/5]rev-list and friends: --min-parents, --max-parents
Date: Wed, 23 Mar 2011 10:48:35 -0400 [thread overview]
Message-ID: <20110323144835.GA30337@sigill.intra.peff.net> (raw)
In-Reply-To: <cover.1300872923.git.git@drmicha.warpmail.net>
On Wed, Mar 23, 2011 at 10:38:47AM +0100, Michael J Gruber wrote:
> Compared to what is currently in pu (which is v2+eps), v3 has:
This one looks good to me (I like the --no-{min,max}-parents).
> 1/3 -> 1/5 unchanged
>
> 2/3 -> 2/5 unchanged
>
> 3/5 is !squash for 2/5 and introduces --no-min-parents and --no-max-parents
> as natural ways to reset the limits
>
> 3/3 -> 4/5 with a fix to the notation in documentation (spell out =<number>)
> and an additional dodecapus test
>
> 5/5 is !fixup for 4/5 and adds test, doc and completion for --no-min-parents
> and --no-max-parents
>
> Junio, please let me/us know whether sending an amended series in this way
> (which I've seen before) is actually convenient for you or not. !squash
> commits require a message edit, for example. OTOH, I don't know any (other?)
> good inter diff solution.
As a reviewer, I find it is usually most convenient to just have the
submitter do the squash, and then write below the "---" (or in a cover
letter like this) a brief explanation of the differences.
Yeah, I end up reading the whole patch again, including bits I already
read, but that is probably a good thing. A good patch is hopefully not
too long, and it is easier (to me, anyway) to review it as a whole and
not as an interdiff.
Technically speaking, what you posted gives more information, and I
could always apply and squash myself to get the final form. But I'm lazy
and I tend to review straight from the mail reader or from $EDITOR while
replying. So to me it's just an extra step.
I have occasionally seen people with long patches reply to the patch
themselves and say "btw, it is hard to see what is changed here in v2,
so here is a much more readable interdiff from v1". And that can be
useful at times, but it's nice to have the patch author using their
judgement about when it will be useful or not.
Just my two cents.
> Thanks for everyone's input which went into this (Junio, Jeff, Jonathan).
Thank you for taking the lead on writing it. :)
-Peff
next prev parent reply other threads:[~2011-03-23 14:48 UTC|newest]
Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-17 11:33 [PATCH/RFD 0/2] revision.c: --merges, --no-merges and --merges-only Michael J Gruber
2011-03-17 11:33 ` [PATCH/RFD 1/2] revision.c: rename --merges to --merges-only Michael J Gruber
2011-03-17 11:33 ` [PATCH/RFD 2/2] revision.c: introduce --merges to undo --no-merges Michael J Gruber
2011-03-17 19:23 ` [PATCH/RFD 0/2] revision.c: --merges, --no-merges and --merges-only Junio C Hamano
2011-03-17 19:59 ` Jeff King
2011-03-18 7:56 ` Michael J Gruber
2011-03-18 8:22 ` Junio C Hamano
2011-03-18 8:41 ` Michael J Gruber
2011-03-18 8:56 ` Jeff King
2011-03-18 14:50 ` [PATCH 0/3] rev-list and friends: --min-parents, --max-parents Michael J Gruber
2011-03-18 14:50 ` [PATCH 1/3] revision.c: introduce --min-parents and --max-parents Michael J Gruber
2011-03-18 19:34 ` Jeff King
2011-03-21 7:31 ` Michael J Gruber
2011-03-18 20:48 ` Jonathan Nieder
2011-03-18 21:21 ` Junio C Hamano
2011-03-21 9:26 ` Michael J Gruber
2011-03-18 14:50 ` [PATCH 2/3] t6009: use test_commit() from test-lib.sh Michael J Gruber
2011-03-18 14:50 ` [PATCH 3/3] rev-list --min-parents,--max-parents: doc and test and completion Michael J Gruber
2011-03-18 19:48 ` Jeff King
2011-03-21 9:01 ` Michael J Gruber
2011-03-21 10:54 ` Jeff King
2011-03-21 12:06 ` Michael J Gruber
2011-03-21 14:54 ` Junio C Hamano
2011-03-21 14:56 ` Michael J Gruber
2011-03-21 16:47 ` Junio C Hamano
2011-03-18 21:14 ` Jonathan Nieder
2011-03-21 8:52 ` Michael J Gruber
2011-03-21 17:49 ` Jonathan Nieder
2011-03-22 7:38 ` Michael J Gruber
2011-03-18 14:54 ` [PATCH 0/3] rev-list and friends: --min-parents, --max-parents Michael J Gruber
2011-03-18 19:41 ` Jeff King
2011-03-21 7:42 ` Michael J Gruber
2011-03-21 10:14 ` [PATCHv2 " Michael J Gruber
2011-03-21 10:14 ` [PATCHv2 1/3] t6009: use test_commit() from test-lib.sh Michael J Gruber
2011-03-21 10:14 ` [PATCHv2 2/3] revision.c: introduce --min-parents and --max-parents Michael J Gruber
2011-03-21 14:04 ` Michael J Gruber
2011-03-21 17:45 ` Junio C Hamano
2011-03-21 17:58 ` Jonathan Nieder
2011-03-21 10:14 ` [PATCHv2 3/3] rev-list --min-parents,--max-parents: doc and test and completion Michael J Gruber
2011-03-21 18:45 ` Jonathan Nieder
2011-03-22 7:55 ` Michael J Gruber
2011-03-23 0:47 ` Jonathan Nieder
2011-03-21 10:56 ` [PATCHv2 0/3] rev-list and friends: --min-parents, --max-parents Jeff King
2011-03-23 9:38 ` [PATCHv3 0/5]rev-list " Michael J Gruber
2011-03-23 9:38 ` [PATCHv3 1/5] t6009: use test_commit() from test-lib.sh Michael J Gruber
2011-03-23 9:38 ` [PATCHv3 2/5] revision.c: introduce --min-parents and --max-parents Michael J Gruber
2011-03-23 9:38 ` [PATCHv3 3/5] squash! " Michael J Gruber
2011-03-23 9:38 ` [PATCHv3 4/5] rev-list --min-parents,--max-parents: doc, test and completion Michael J Gruber
2011-03-23 9:38 ` [PATCHv3 5/5] fixup! " Michael J Gruber
2011-03-23 14:48 ` Jeff King [this message]
2011-03-23 17:12 ` [PATCHv3 0/5]rev-list and friends: --min-parents, --max-parents Junio C Hamano
2011-03-23 18:06 ` Junio C Hamano
2011-03-24 8:21 ` Jonathan Nieder
2011-03-24 8:55 ` Michael J Gruber
2011-03-24 9:42 ` Jonathan Nieder
2011-08-08 1:13 ` [PATCH/RFC 0/2] test_when_finished and returning early Jonathan Nieder
2011-08-08 1:15 ` [PATCH 1/2] test: simplify return value of test_run_ Jonathan Nieder
2011-08-08 1:17 ` [PATCH 2/2] test: cope better with use of return for errors Jonathan Nieder
2011-08-09 8:46 ` Johannes Sixt
2011-08-09 15:36 ` Jeff King
2011-08-11 7:05 ` [PATCH v2] t3900: do not reference numbered arguments from the test script Johannes Sixt
2011-08-11 7:11 ` Jonathan Nieder
2011-08-11 21:49 ` Junio C Hamano
2011-08-08 1:26 ` [PATCH/RFC 0/2] test_when_finished and returning early Jeff King
2011-03-18 9:07 ` [PATCH/RFD 0/2] revision.c: --merges, --no-merges and --merges-only Jeff King
2011-03-18 9:42 ` Michael J Gruber
2011-03-18 9:54 ` Jeff King
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=20110323144835.GA30337@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@drmicha.warpmail.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
/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).