Re: [PATCH] git-send-email.perl: implement suggestions made by perlcritic

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Mar 21, 2013, 16:51 UTC
Message-ID: <CALkWK0kkWAkiANL-vL-OthKSDkoU2b_Q68frkGc06QNG7RbSFQ@mail.gmail.com>
In-Reply-To: <7vd2usbwkn.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

Show 21 quoted lines

> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index be809e5..e974b11 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -513,7 +513,7 @@ if (@alias_files and $aliasfiletype and defined $parse_alias{$aliasfiletype}) {
>>  ($sender) = expand_aliases($sender) if defined $sender;
>>
>>  # returns 1 if the conflict must be solved using it as a format-patch argument
>> -sub check_file_rev_conflict($) {
>> +sub check_file_rev_conflict {
>
> Have you verified that the callers of this sub are OK with this
> change?  It used to force scalar context on its arguments but now it
> does not.
>
> I am not saying I know the callers will get broken.  I am trying to
> make sure that this is *not* the result of blindly following
> perlcritic output without understanding the ramifications of the
> change.

No Junio, I haven't blindly followed the perlcritic output. I've considered the ramifications, audited the callers, and run the tests to make sure that nothing breaks.

Show 8 quoted lines

>> @@ -1438,7 +1438,7 @@ sub recipients_cmd {
>>
>>       my $sanitized_sender = sanitize_address($sender);
>>       my @addresses = ();
>> -     open my $fh, "$cmd \Q$file\E |"
>> +     open my $fh, q{-|}, "$cmd \Q$file\E"
>
> Strange quoting (why not just say "-|"?) aside

Intentional. I thought it was a pretty way to differentiate between a mode string (which can only be <, >, or -|) and a filename string. If you don't share my taste, feel free to use quotes instead.

Show 5 quoted lines

> , if you are moving
> away from the two-param form of open(), it would be a sane thing to
> do to also stop concatenating the arguments to commands to avoid
> shell metacharacter gotcha.  It will make the resulting code much
> safer.

Yes, the file can be improved in many ways, but that is not the topic of this series. This series just makes the changes suggested by perlcritic gentle.