Re: [PATCH] git-send-email.perl: implement suggestions made by perlcritic
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:56:28
Ramkumar Ramachandra [off-list ref] writes:
quoted hunk
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.
quoted hunk
@@ -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, 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.