Subject: Re: [PATCH v2 2/4] git-send-email: refactor duplicate $? checks into a function

From: Junio C Hamano <[email-masked]>
Date: Mon, 05 Apr 2021 16:47:09 -0700

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> Refactor the duplicate checking of $? into a function. There's an
> outstanding series[1] wanting to add a third use of system() in this
> file, let's not copy this boilerplate anymore when that happens.
>
> 1. http://lore.kernel.org/git/[email-masked]
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
> ---
>  git-send-email.perl | 48 +++++++++++++++++++++++++++++----------------
>  1 file changed, 31 insertions(+), 17 deletions(-)

>  sub validate_patch {
>  	my ($fn, $xfer_encoding) = @_;
>  
> @@ -1952,11 +1965,12 @@ sub validate_patch {
>  			chdir($repo->wc_path() or $repo->repo_path())
>  				or die("chdir: $!");
>  			local $ENV{"GIT_DIR"} = $repo->repo_path();
> -			$hook_error = "rejected by sendemail-validate hook"
> -				if system($validate_hook, $target);
> +			if (my $msg = system_or_msg([$validate_hook, $target])) {
> +				$hook_error = __("rejected by sendemail-validate hook");
> +			}
>  			chdir($cwd_save) or die("chdir: $!");
>  		}
> -		return $hook_error if $hook_error;
> +		validate_patch_error($fn, $hook_error) if $hook_error;
>  	}

One big thing that is different between this version and the one in
Emily's "config hook" topic is that this is still limited to the
case where $repo exists.  In the new world order, it will not matter
in what directory the command runs, as long as "git hook" finds the
hook, and details of the invocation is hidden behind the command.

I presume that Emily's series is expected to be updated soonish?
Please figure out who to go first and other details to work well
together between you two.

I'd drop the "config hook" topic for now, and I think the endpoint
of these four-patch series (the first "map vs for" can move more or
less independently) are more-or-less in a good shape (even though as
I said already, I think 2/4 and 4/4 want to be updated not to
introduce the intermediate "validate_patch_error()" sub in 2/4 only
to get rid of it in 4/4) and would require only one update.

Thanks.