From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Thu, 14 Jun 2012 19:07:20 GMT
Subject: Re: [eclipse7@gmx.net: [PATCH] diff: Only count lines in show_shortstats()]
Message-ID: <4FDA3668.3000900@in.waw.pl>
In-Reply-To: <7vk3zig92n.fsf@alter.siamese.dyndns.org>

On 06/07/2012 10:29 PM, Junio C Hamano wrote:
>>>> >> > --- a/t/t4012-diff-binary.sh
>>>> >> > +++ b/t/t4012-diff-binary.sh
>>>> >> > @@ -36,6 +36,14 @@ test_expect_success '"apply --stat" output for binary file change' '
>>>> >> >  	test_i18ncmp expected current
>>>> >> >  '
>>>> >> >  
>>>> >> > +cat > expected <<\EOF
>>>> >> > + 4 files changed, 2 insertions(+), 2 deletions(-)
>>>> >> > +EOF
>>>> >> > +test_expect_success 'diff with --shortstat' '
>>>> >> > +	git diff --shortstat >current &&
>>>> >> > +	test_cmp expected current
>>>> >> > +'
>>>> >> > +
>>> >> The test is OK, and follows the style of surrounding tests, but current
>>> >> style is slightly different:
>>> >> - no space after '>'
>>> >> - expected output is inlined if it is short
>>> >> - test_i18ncmp is used, even if the message is not yet i18n-ized
>>> >> 
>>> >> Something like this:
>>> >> test_expect_success 'diff --shortstat output for binary file change' '
>>> >> 	echo " 4 files changed, 2 insertions(+), 2 deletions(-)" >expect &&
>>> >> 	git diff --shortstat >current &&
>>> >> 	test_i18ncmp expect current
>>> >> '
>> >
>> >   Should I rewrite the test for this patch? Or should it be changed for the
>> > whole file at once?
> Please keep a bugfix patch to only fixes with tests.  Style fixes
> should be done later after dust from more important changes (e.g. a
> bugfix) settles.
> 
> Thanks.
Does this need a v2?

Zbyszek