Re: [PATCH] contacts: introduce --since and --min-percent

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jan 16, 2015, 21:11 UTC
Message-ID: <CAPig+cS5is73damiYgCmWp3QzmBX+E38f26oyn2nwG0FO2AStQ@mail.gmail.com>
In-Reply-To: <1421441933-54183-1-git-send-email-artagnon@gmail.com>

On Fri, Jan 16, 2015 at 3:58 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

Show 11 quoted lines

> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> diff --git a/contrib/contacts/git-contacts b/contrib/contacts/git-contacts
> index dbe2abf..b06f2e1 100755
> --- a/contrib/contacts/git-contacts
> +++ b/contrib/contacts/git-contacts
> @@ -8,12 +8,16 @@
>  use strict;
>  use warnings;
>  use IPC::Open2;
> +use Getopt::Long qw/:config gnu_getopt no_ignore_case auto_abbrev/;

Most of the rest of the codebase uses qw(...) rather than qw/.../.

Show 7 quoted lines

>  my $since = '5-years-ago';
>  my $min_percent = 10;
>  my $labels_rx = qr/Signed-off-by|Reviewed-by|Acked-by|Cc/i;
>  my %seen;
>
> +my $rv = GetOptions('since=s' => \$since, 'min-percent=i' => \$min_percent);
> +exit 1 if (!$rv);

This would make more sense if moved down to the point where the script arguments are processed (just before the 'if (!@ARGV)' line, for instance).

These new options should be documented in git-contacts.txt. Also, the "Limitations" section of the documentation says that these values are currently hard-coded, so it deserves an update as well.

Show 5 quoted lines

>  sub format_contact {
>         my ($name, $email) = @_;
>         return "$name <$email>";
> --
> 2.2.1