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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Mar 21, 2011, 17:49 UTC
Message-ID: <20110321174917.GA8202@elie>
In-Reply-To: <4D8711DD.2040407@drmicha.warpmail.net>

Michael J Gruber wrote:

> Come on, with a cover-letter saying doc and tests are in 3/3, how much
> effort is it to read that before 1/3 if you care?

Nonzero. Now multiply that by the number of people who are going to look at the history over the years.

> The tests are bogus before the code and the doc pointless before it.
>
> Squashing 1 and 3 is okay, of course. For my own digestion, smaller
> bites are better.

The reason I mentioned the possibility of docs and tests before implementation is that that can be a good way to get feedback on the design of something when the implementation is not even ready yet. Which doesn't apply hear, of course.

Anyway, I don't care too much about this, but I wanted to make the convention clear (if I have understood it correctly, at least).

>>    Avoiding the for loop means errors from 'echo' before the last
>>    iteration are not ignored; a more verbose way to write the same
>
> Do we really need to safe-guard echo and prints?

No; sorry about that. I get worried about for loops because they have a tendency to start using git commands later, but I shouldn't have mentioned it.

> Thanks for both of the above, that makes things much better. Although I
> have to treat the case with empty rev-list specially now, or use the
> verbose version.

Sorry I missed it; good catch.  I don't recommend

	printf ${1+'%s\n' "$@"}

> Dodeka, really? I leave that to you.
> I might add a tetrapus, though.

I meant "greater than eight", since it seemed like a test that in some alternate universe could fail. I'll write a test on top.

(Dodecapus comes from [1].) http://thread.gmane.org/gmane.comp.version-control.git/15486 )I mentioned dodecapus

> wins against? Against itself, i.e. it overrides previous occurences. But
> I'll separate these.

Thanks.  The details weren't important.

Show 6 quoted lines

>> 	test_expect_failure '--max-parents=gobbledegood errors out' '
>> 		...
>> 	'
>
> I don't really want to parse for the string "infinity" nor go for strtol
> instead of atoi. Why shouldn't something "unparseable" be 0?

Three reasons:

 1) why shouldn't it be -1 or 27?  When in doubt, it's best to let
    the operator know so she can specify what is wanted unambiguously.
 2) to catch typos on the commandline and bugs in scripts
 3) less importantly, erroring out makes it less likely someone is
    going to rely on it (since we didn't point out their typo) and
    thus makes future extensions easier

Thanks again. Now to look at v2. :) Jonathan

[1] http://thread.gmane.org/gmane.comp.version-control.git/15486