From: Junio C Hamano <gitster@pobox.com>
To: Alexander Strasser <eclipse7@gmx.net>
Cc: "Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>,
"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, 07 Jun 2012 13:29:04 -0700 [thread overview]
Message-ID: <7vk3zig92n.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20120607200434.GA2965@akuma> (Alexander Strasser's message of "Thu, 7 Jun 2012 22:04:34 +0200")
Alexander Strasser <eclipse7@gmx.net> writes:
Administrivia.
Please do not use Mail-Followup-To header to deflect direct response
to you away to other people. When I want to reply to you and Cc
others, I do not want to see other people's name on To field for me
to edit and correct, and when somebody else wants to reply to you, I
do not want to see my name on its To line, as such a message may not
be of immediate interest for me.
> Zbigniew Jędrzejewski-Szmek wrote:
> ...
>> > I think I should have put you in CC. But I am not so sure about
>> > Git patch submission policies.
>> The policy is to CC everyone who might be interested, and also to add
>> TO:gitster@pobox.com, if the patch is intended for merging, as yours is.
Correction. It is not "is intended for merging", but only when it
is *ready* to be merged, when stakeholders are happy with the patch.
>> So basically taking the address list from the discussion of e18872b
>> would be the simplest and most effective choice.
> Yes, seems so. I was seeing changing line counts in GitStats output
> compared to older and newer Git versions. I found the exact commit with
> "git bisect" which was a big help.
Thanks.
>> > Signed-off-by: Alexander Strasser <eclipse7@gmx.net>
>> Small note: normally the paragraphs are not indented.
>
> Noted. I probably should have also dropped the () in the subject. After
> submitting I noticed this notation was not used in analog log messages.
>
> [...]
>> > --- 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.
next prev parent reply other threads:[~2012-06-07 20:29 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 [this message]
2012-06-14 19:07 ` Zbigniew Jędrzejewski-Szmek
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=7vk3zig92n.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=eclipse7@gmx.net \
--cc=git@vger.kernel.org \
--cc=j.sixt@viscovery.net \
--cc=mj@ucw.cz \
--cc=zbyszek@in.waw.pl \
/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).