Re: [RFC] git: make signoff header configurable

[Junio C Hamano <gitster@pobox.com>]

"Arend van Spriel" <arend@broadcom.com> writes:

> I had an itch to scratch. Like the -s command line parameter to
> get the signed-off message added, but not all projects use the
> same signature format. So let me know what you think about this
> idea. Never contributed to git before so decided to make it an
> RFC first as this solution may be a bit hack-ish.

It is customary to declare things like "char *get_signoff_header()"
that are meant to be available pretty much everywhere in cache.h
(look for /* Environment bits from configuration mechanism */) and
not in an unrelated header like sequencer.h (which builtin/commit.c
or config.c have no business including).  Also, default-config may
be a bit too generic place to read this information; it is used by
many read-only commands like "log", "diff", etc. that have no reason
to know this custom trailer setting.

Other than these, I do not see anything glaringly "hack-ish" in the
implementation itself.  Of course, a non-RFC patch would come with
documentation and test script updates.

By the way, what you are adding is a trailer, not a header, as it
comes at the very end ;-).

To projects that adopt the S-o-b convention from the kernel, the act
of signing off has a very specific legal meaning (I know it is not
an electronic signature, but the intention counts in court); I am
not sure if it is even a good idea to make "-s" mean something
different depending on the configuration in the first place, though.

Wouldn't a commit template a better alternative for appending a
random stuff in the log message, I wonder.

>  builtin/commit.c |    5 +++--
>  config.c         |   12 ++++++++++++
>  sequencer.c      |   15 +++++++++++++--
>  sequencer.h      |    3 ++-
>  4 files changed, 30 insertions(+), 5 deletions(-)
> diff --git a/sequencer.h b/sequencer.h
> index 1fc22dc..5a91105 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -48,8 +48,9 @@ struct replay_opts {
>  
>  int sequencer_pick_revisions(struct replay_opts *opts);
>  
> -extern const char sign_off_header[];
> +extern char* sign_off_header;

The asterisk sticks to the identifier, not type, i.e.

	extern char *signoff_label;

But I suspect you do not need to make this extern (for the same
reason we can keep def_signoff_header[] a static), as long as the
public facing API "get-signoff-header" is extern.

> +char *get_signoff_header(void);
>  void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);
>  
>  #endif