Re: [PATCHv3 0/5]rev-list and friends: --min-parents, --max-parents

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]