Re: [RFC] git: make signoff header configurable
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:57:09
"Arend van Spriel" [off-list ref] 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.
quoted hunk
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