Re: [PATCH v2] send-email: support validate hook

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes:

> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index 706091a56..b2514f4d4 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -447,6 +447,14 @@ rebase::
>  The commits are guaranteed to be listed in the order that they were
>  processed by rebase.
>  
> +sendemail-validate
> +~~~~~~~~~~~~~~~~~~
> +
> +This hook is invoked by 'git send-email'.  It takes a single parameter,
> +the name of the file that holds the e-mail to be sent.  Exiting with a
> +non-zero status causes 'git send-email' to abort before sending any
> +e-mails.
> +

I briefly wondered if an interface that allows only the name of the
file will close the door to future enhancements, but in the end
decided it is OK.  E.g. users may want "here is the contents, is it
appropriate to be sent to this and that address?"---but this hook is
meant to enhances/extends the existing "make sure the contents do
not syntactically bust the technical limit of underlying transport",
and sits at a place best to do so in the codeflow, i.e. before we
even start to decide who the recipients of the patch are.  So those
who want "given the contents of the patch and list of the recipients,
decide if it is OK to send the patch to all of them" would be better
served by a separate hook, not this one.

> +	write_script .git/hooks/sendemail-validate <<-\EOF &&
> +		# test that we have the correct environment variable, pwd, and
> +		# argument
> +		case "$GIT_DIR" in
> +			*.git)
> +				true
> +				;;
> +			*)
> +				false
> +				;;
> +		esac &&
> +		test -e 0001-add-master.patch &&
> +		grep "add master" "$1"
> +	EOF

I'd squash in cosmetic changes to de-dent the contents of
write_script (our standard style is that the body of the script is
written as if the column at which write_script..EOF starts is the
left-edge of the page; I think this file already has a few style
violations that may want to be updated with a separate clean-up
patch when the file is quiet), and then de-dent the case arm (case
arms are indented at the same depth as case/esac).  Also I think we
care that 0001-add-master.patch not just exists but is a file, so
I'd do s/test -e/test -f/ while at it.

Thanks.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]