Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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/87y2esg22j.fsf@xxxxxxxxxxxxxxxxxxx > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> > --- > 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.
