From: Junio C Hamano <gitster@pobox•com>
To: Jonathan Tan <jonathantanmy@google•com>
Cc: git@vger•kernel.org, avarab@gmail•com
Subject: Re: [PATCH v2] send-email: support validate hook
Date: Mon, 15 May 2017 10:55:52 +0900 [thread overview]
Message-ID: <xmqqk25iyjbr.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170512223827.21372-1-jonathantanmy@google.com> (Jonathan Tan's message of "Fri, 12 May 2017 15:38:26 -0700")
Jonathan Tan <jonathantanmy@google•com> 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.
next prev parent reply other threads:[~2017-05-15 1:55 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-11 19:37 [RFC] send-email: support validate hook Jonathan Tan
2017-05-11 20:39 ` Brandon Williams
2017-05-12 2:15 ` Junio C Hamano
2017-05-12 7:23 ` Ævar Arnfjörð Bjarmason
2017-05-12 22:31 ` Jonathan Tan
2017-05-12 22:34 ` Ævar Arnfjörð Bjarmason
2017-05-12 22:38 ` [PATCH v2] " Jonathan Tan
2017-05-15 1:55 ` Junio C Hamano [this message]
2017-05-15 18:10 ` Jonathan Tan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqk25iyjbr.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=avarab@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=jonathantanmy@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox