git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>
To: Junio C Hamano <gitster@pobox.com>
Cc: Alexander Strasser <eclipse7@gmx.net>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	mj@ucw.cz, Johannes Sixt <j.sixt@viscovery.net>
Subject: Re: [eclipse7@gmx.net: [PATCH] diff: Only count lines in show_shortstats()]
Date: Thu, 14 Jun 2012 21:07:20 +0200	[thread overview]
Message-ID: <4FDA3668.3000900@in.waw.pl> (raw)
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

  reply	other threads:[~2012-06-14 19:07 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20120607122149.GA3070@akuma>
2012-06-07 19:05 ` [eclipse7@gmx.net: [PATCH] diff: Only count lines in show_shortstats()] Zbigniew Jędrzejewski-Szmek
2012-06-07 20:04   ` Alexander Strasser
2012-06-07 20:29     ` Junio C Hamano
2012-06-14 19:07       ` Zbigniew Jędrzejewski-Szmek [this message]
2012-06-14 20:28         ` Junio C Hamano

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=4FDA3668.3000900@in.waw.pl \
    --to=zbyszek@in.waw.pl \
    --cc=eclipse7@gmx.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j.sixt@viscovery.net \
    --cc=mj@ucw.cz \
    /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).