git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "mataha via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, mataha <mateusz.kazimierczuk@xolv.io>
Subject: Re: [PATCH] templates: clarify SHA1 arg in prepare-commit-msg
Date: Wed, 18 May 2022 11:58:46 -0700	[thread overview]
Message-ID: <xmqqtu9mvfa1.fsf@gitster.g> (raw)
In-Reply-To: <pull.1265.git.git.1652899219597.gitgitgadget@gmail.com> (mataha via GitGitGadget's message of "Wed, 18 May 2022 18:40:19 +0000")

Welcome to the development community.

"mataha via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: mataha <mateusz.kazimierczuk@xolv.io>

The name here must match the name you sign the patch off as,
i.e. this line should read

    From: Mateusz 'mataha' Kazimierczuk <mateusz.kazimierczuk@xolv.io>

> 'prepare-commit-msg' hook sample description doesn't mention the third
> argument (a commit object name) nor when is it actually passed to that
> hook by git-commit (if the source is a commit; see builtin/commit.c#L777,
> sequencer.c#L1219). Seeing that it's documented in githooks(5), there
> should be no reason not to include that in the sample hook as well.

There should be a mention of the commit that added the hook, namely,
8089c85b (git-commit: add a prepare-commit-msg hook, 2008-02-05).
The log message for this patch should also observe that this third
argument, which is given only when the source is 'commit', was not
mentioned in the sample hook added by the original commit.

Interestingly, its log message reads:

    ...
    Its purpose is to modify the commit message in-place.

    It takes one to three parameters.  The first is the name of the file that
    the commit log message.  The second is the source of the commit message,
    and can be: "message" (if a -m or -F option was given); "template" (if a
    -t option was given or the configuration option commit.template is set);
    "merge" (if the commit is a merge or a .git/MERGE_MSG file exists);
    "squash" (if a .git/SQUASH_MSG file exists); or "commit", followed by
    a commit SHA1 as the third parameter (if a -c, -C or --amend option
    was given).
    ...

So it wasn't a mistake that the sample did not mention it.  I think
the reason why the original, and today's version, does not mention
it is because no sample code in the file actually uses it.

I am not yet convinced that it is a good idea to only add mention to
the third one if we are not showing how it is used to this file,
especially given that in the documentation we _do_ cover these
arguments fully.

Thanks.

> Signed-off-by: Mateusz 'mataha' Kazimierczuk <mateusz.kazimierczuk@xolv.io>
> ---
>     Mention the third argument in 'prepare-commit-msg' hook sample
>     
>     'prepare-commit-msg' hook sample doesn't mention what the third argument
>     is for nor when is it actually passed; I feel like it should be, for the
>     sake of convenience (this doesn't mean that a user shouldn't refer to a
>     more detailed description in the manual, of course).
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1265%2Fmataha%2Fdoc%2Fprepare-commit-msg-args-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1265/mataha/doc/prepare-commit-msg-args-v1
> Pull-Request: https://github.com/git/git/pull/1265
>
>  templates/hooks--prepare-commit-msg.sample | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
> index 318afe3fd86..bc06d0701a8 100755
> --- a/templates/hooks--prepare-commit-msg.sample
> +++ b/templates/hooks--prepare-commit-msg.sample
> @@ -3,9 +3,9 @@
>  # An example hook script to prepare the commit log message.
>  # Called by "git commit" with the name of the file that has the
>  # commit message, followed by the description of the commit
> -# message's source.  The hook's purpose is to edit the commit
> -# message file.  If the hook fails with a non-zero status,
> -# the commit is aborted.
> +# message's source and the commit object name (if the source was
> +# a commit).  The hook's purpose is to edit the commit message file.
> +# If the hook fails with a non-zero status, the commit is aborted.
>  #
>  # To enable this hook, rename this file to "prepare-commit-msg".
>  
>
> base-commit: 277cf0bc36094f6dc4297d8c9cef79df045b735d

      reply	other threads:[~2022-05-18 18:59 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-18 18:40 [PATCH] templates: clarify SHA1 arg in prepare-commit-msg mataha via GitGitGadget
2022-05-18 18:58 ` Junio C Hamano [this message]

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqtu9mvfa1.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=mateusz.kazimierczuk@xolv.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).