Subject: Re: [PATCHv2 3/3] rev-list --min-parents,--max-parents: doc and test and completion

From: Jonathan Nieder <[email-masked]>
Date: Tue, 22 Mar 2011 19:47:30 -0500

Michael J Gruber wrote:
> Jonathan Nieder venit, vidit, dixit 21.03.2011 19:45:
>> Michael J Gruber wrote:

>>> Based on mg/rev-list-one-side-only (in next) to save Junio a build conflict
>>> resolution.
>>
>> Not a serious problem, but I wish it hadn't been.  What particular
>
> But why? Basing it on something earlier would have served no purpose
> (that I know of) at all.
>
> I should have mentioned the dependency in v1.

*nod*  Sorry, I was just confused before --- I hadn't understood why
you were doing it.  Giving a heads up like you did about interaction
between topics is indeed very useful.

To answer your question about why: basing it on master would mean
that Junio could merge it to master without merging
mg/rev-list-one-side-only first.  Similarly, I had wanted to test
against master because master is what I use day-to-day so applying
it there would avoid confusing unrelated changes.

>> Seems hackish.  Maybe --no-max-parents could denote infinity?
>
> For me, "-1" is a quite natural way to reset a count type parameter

Natural to me in code, but not on the command line.  I can write a
separate patch for --no-max-parents if you'd like.

> There is no problem parsing for "--max-parents=infinity" and/or
> "--no-max-parents" or even (better?) "--max-parents=" without number,
> it's only a matter of bike shedding decisions.

Well, if it doesn't matter what color it is, I guess I shouldn't have
mentioned it then?

> Hmmm, are there whitespace issues which am warns about and diff does
> not, or have I missed a warning?

Yep, diff doesn't warn about anything without --check.

>>> +test_expect_success 'rev-list override and infinities' '
>>> +
>>> +	check_revlist "--min-parents=2 --max-parents=1 --max-parents=3" tripus normalmerge &&
>>> +	check_revlist "--min-parents=1 --min-parents=2 --max-parents=7" tetrapus tripus normalmerge &&
>>> +	check_revlist "--min-parents=2 --max-parents=8" tetrapus tripus normalmerge &&
>>> +	check_revlist "--min-parents=2 --max-parents=-1" tetrapus tripus normalmerge
>>> +'
>> 
>> 7 and 8 don't mean infinity any more, do they?  What is this test
>> checking?
>
> The test checks "override and infinities", where the plural indicates
> the fact that it tests different ways of spelling (practical) infinity
> such as the very suggestive "8" which nobody cares about but me.

Ah, so that's what you meant.  git.git has very few octopus merges,
so even "4" is a practical infinity there (well, technically there's
an early 6-parent commit).

Regards,
Jonathan