git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: Kevin Ballard <kevin@sb.org>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] Make test script annotate-tests.sh handle missing authors
Date: Sat, 16 Oct 2010 14:22:16 +0200	[thread overview]
Message-ID: <201010161422.17483.jnareb@gmail.com> (raw)
In-Reply-To: <09193539-B5AD-4574-9FE4-983566A34355@sb.org>

On Sat, 16 Oct 2010, Kevin Ballard wrote:
> On Oct 16, 2010, at 12:34 AM, Jakub Narebski wrote:
>> Kevin Ballard <kevin@sb.org> writes:
>> 
>>> Also, I'm not a Perl programmer, so it's possible there's a better idiom
>>> for this sort of thing.
>>> 
>>> t/annotate-tests.sh |    3 +++
>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>> 
>>> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
>>> index 396b965..4e37a66 100644
>>> --- a/t/annotate-tests.sh
>>> +++ b/t/annotate-tests.sh
>>> @@ -9,6 +9,9 @@ check_count () {
>>> 	cat .result | perl -e '
>>> 		my %expect = (@ARGV);
>>> 		my %count = ();
>>> +		while (my ($author, $count) = each %expect) {
>>> +			$count{$author} = 0;
>>> +		}
>> 
>> 
>> First, it is a very bad practice to have variables of different type
>> named the same way, here %count (hash) and $count (scalar, unused).
> 
> Thanks for the pointer, but $count is already used in the while loop below:
> 
> 		while (my ($author, $count) = each %count) {
> 			my $ok;
> 			if ($expect{$author} != $count) {
> 				$bad = 1;
> 				$ok = "bad";
> 			}
> 			else {
> 				$ok = "good";
> 			}
> 			print STDERR "Author $author (expected $expect{$author}, attributed $count) $ok\n";
> 		}

Hmmm... the %count hash should probably be named %actual (to complement
%expect), or %attributed (like in output).

-- 
Jakub Narebski
Poland

      parent reply	other threads:[~2010-10-16 13:23 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-16  5:50 [PATCH] Make test script annotate-tests.sh handle missing authors Kevin Ballard
2010-10-16  7:34 ` Jakub Narebski
2010-10-16 10:43   ` Kevin Ballard
2010-10-16 11:09     ` [PATCH v2] Update test script annotate-tests.sh to handle missing/extra authors Kevin Ballard
2010-10-16 12:22     ` Jakub Narebski [this message]

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=201010161422.17483.jnareb@gmail.com \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kevin@sb.org \
    /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).