Re: [PATCH] commit: Add commit.signoff configuration option

["D. Ben Knoble" <ben.knoble@gmail.com>]

On Tue, May 13, 2025 at 8:21 AM Chris Down <chris@chrisdown.name> wrote:
>
> Introduce a new `commit.signoff` config variable that mirrors the
> behavior of the -s/--signoff flag.
>
> We already have prior art in format-patch with `format.signoff`; this
> brings parity for those who don’t use a patch-based workflow but still
> rely on signoffs.
>
> Right now people who have to do this regularly often alias commit to
> `commit --signoff` in their shell, which is less than ideal -- this
> config option avoids having to do that.

It would probably be nice to say why this makes sense in light of
previously-raised objections, too [1].

[1]: https://lore.kernel.org/git/xmqqo6x4p6z2.fsf@gitster.g/

>
> Signed-off-by: Chris Down <chris@chrisdown.name>
> ---
>  Documentation/signoff-option.adoc         |  4 ++++
>  builtin/commit.c                          |  4 ++++
>  t/t7500-commit-template-squash-signoff.sh | 10 ++++++++++
>  3 files changed, 18 insertions(+)
>
> diff --git a/Documentation/signoff-option.adoc b/Documentation/signoff-option.adoc
> index cddfb225d1..0055874e84 100644
> --- a/Documentation/signoff-option.adoc
> +++ b/Documentation/signoff-option.adoc
> @@ -13,6 +13,10 @@ endif::git-commit[]
>         Linux kernel and Git projects.)  Consult the documentation or
>         leadership of the project to which you're contributing to
>         understand how the signoffs are used in that project.
> +ifdef::git-commit[]
> +       The `commit.signoff` configuration variable may also be used to imply
> +       `--signoff`.
> +endif::git-commit[]
>  +
>  The `--no-signoff` option can be used to countermand an earlier `--signoff`
>  option on the command line.
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 66bd91fd52..da98895438 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1670,6 +1670,10 @@ static int git_commit_config(const char *k, const char *v,
>                                                                &is_bool);
>                 return 0;
>         }
> +       if (!strcmp(k, "commit.signoff")) {
> +               signoff = git_config_bool(k, v);
> +               return 0;
> +       }
>
>         return git_status_config(k, v, ctx, s);
>  }
> diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh
> index 4dca8d97a7..03c20dcb1d 100755
> --- a/t/t7500-commit-template-squash-signoff.sh
> +++ b/t/t7500-commit-template-squash-signoff.sh
> @@ -181,6 +181,16 @@ test_expect_success '--signoff' '
>         test_cmp expect output
>  '
>
> +test_expect_success 'config commit.signoff implies signoff' '
> +       git config commit.signoff true &&

This should use test_config, I think. Otherwise, the config will stick
around if the test fails. You /could/ use test_when_finished, but
test_config sets that up for you.

> +       echo "871119" >> bar &&
> +       git add bar &&
> +       echo "zort" | git commit -F - bar &&
> +       git cat-file commit HEAD | sed "1,/^\$/d" > output &&
> +       test_cmp expect output &&

Took me a bit to realize (since it doesn't show in the context);
"expect" is setup outside the previous signoff test and then (re)used
here. Reasonable, but we now have commit_msg_is in this test script.
Perhaps worth a quick refactor patch preceding this one to make it use
that function? Probably not required, though?

> +       git config --unset commit.signoff
> +'
> +
>  test_expect_success 'commit message from file (1)' '
>         mkdir subdir &&
>         echo "Log in top directory" >log &&
> --
> 2.49.0
>
>


-- 
D. Ben Knoble