Re: [PATCH v6 2/2] send-email: expose header information to git-send-email's sendemail-validate hook

[Luben Tuikov <luben.tuikov@amd.com>]

Hi Michael,

Good work on this. I've a few tiny notes following.

On 2023-01-16 20:39, Strawbridge, Michael wrote:
> To allow further flexibility in the git hook, the SMTP header

"git" is something different. You want to use the capitalization "Git".

> information of the email that git-send-email intends to send, is now

"that" --> "which".

> passed as a 2nd argument to the sendemail-validate hook.

"a 2nd argument" --> "the 2nd argument".
 
> As an example, this can be useful for acting upon keywords in the
> subject or specific email addresses.
> 
> Cc: Luben Tuikov <luben.tuikov@amd.com>
> Cc: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
> ---
>  Documentation/githooks.txt | 29 +++++++++++++++++++----
>  git-send-email.perl        | 31 +++++++++++++++++--------
>  t/t9001-send-email.sh      | 47 ++++++++++++++++++++++++++++++++++++--
>  3 files changed, 91 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index a16e62bc8c..e80f481efd 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -583,10 +583,31 @@ processed by rebase.
>  sendemail-validate
>  ~~~~~~~~~~~~~~~~~~
>  
> -This hook is invoked by linkgit:git-send-email[1].  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.
> +This hook is invoked by linkgit:git-send-email[1].
> +
> +It takes these command line arguments:

"It takes two command line arguments. They are,"

> +1. the name of the file that holds the e-mail to be sent.

"which holds the contents of the email to be sent."
Sentence ends and the next one should be capitalized.

> +2. the name of the file that holds the SMTP headers to be used.

"The name of the file which holds the SMTP envelope and headers of the email."

> +
> +The SMTP headers will be passed to the hook in the below format.
> +Take notice of the capitalization and multi-line tab structure.

Always use present simple tense when describing mechanics of code,
not future tense, "are passed". Think of when the user is reading
this long after the patch went in.

Also, please use "the format below."

> +  From: Example <from@example.com>
> +  To: to@example.com
> +  Cc: cc@example.com,
> +	  A <author@example.com>,
> +	  One <one@example.com>,
> +	  two@example.com
> +  Subject: PATCH-STRING
> +  Date: DATE-STRING
> +  Message-Id: MESSAGE-ID-STRING
> +  X-Mailer: X-MAILER-STRING
> +  Reply-To: Reply <reply@example.com>
> +  MIME-Version: 1.0
> +  Content-Transfer-Encoding: quoted-printable

Perhaps this is too much detail and unnecessary for the generalization
we're trying to achieve here?

Maybe the following would suffice?

   The SMTP envelope and headers are passed as the 2nd argument to the
   hook, exactly as they are passed to the user's Mail Transport Agent (MTA).
   In effect, the email given to the user's MTA, is the contents of $2 followed
   by the contents of $1.
-- 
Regards,
Luben