{"thread":{"id":"51993","subject":"[PATCH 1/1] commit: add support to provide --coauthor","startedAt":"2019-10-08T07:55:07Z","lastAt":"2019-10-11T04:09:51Z","messageCount":15,"participants":["Toon Claes","Denton Liu","Phillip Wood","SZEDER Gábor","Junio C Hamano","brian m. carlson","Jeff King","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"383660","messageId":"20191008074935.10972-1-toon@iotcl.com","threadId":"51993","inReplyTo":null,"subject":"[PATCH 1/1] commit: add support to provide --coauthor","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2019-10-08T07:49:35Z","receivedAt":"2019-10-08T07:55:07Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Add support to provide the Co-author when committing. For each\nco-author provided with --coauthor=<coauthor>, a line is added at the\nbottom of the commit message, like this:\n\n    Co-authored-by: <coauthor>\n\nIt's a common practice use when pairing up with other people and both\nauthors want to in the commit message.\n\nCo-authored-by: Zeger-Jan van de Weg <git@zjvandeweg.nl>\nSigned-off-by: Toon Claes <toon@iotcl.com>\n---\n Documentation/git-commit.txt |  5 ++++\n builtin/commit.c             |  7 ++++++\n sequencer.c                  | 44 ++++++++++++++++++++++++++----------\n sequencer.h                  |  2 ++\n t/t7502-commit-porcelain.sh  | 11 +++++++++\n 5 files changed, 57 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex afa7b75a23..c059944e38 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -140,6 +140,11 @@ OPTIONS\n \tcommit by that author (i.e. rev-list --all -i --author=<author>);\n \tthe commit author is then copied from the first such commit found.\n\n+--coauthor=<coauthor>::\n+        Add a Co-authored-by line with the specified author. Specify the\n+\tauthor using the standard `Co Artur <co-artur@example.com>`\n+\tformat.\n+\n --date=<date>::\n \tOverride the author date used in the commit.\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex ae7aaf6dc6..feb423ed6f 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -110,6 +110,7 @@ static int config_commit_verbose = -1; /* unspecified */\n static int no_post_rewrite, allow_empty_message;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n static char *sign_commit;\n+static struct string_list coauthors = STRING_LIST_INIT_NODUP;\n\n /*\n  * The default commit message cleanup mode will remove the lines\n@@ -672,6 +673,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \tint clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE);\n \tint old_display_comment_prefix;\n \tint merge_contains_scissors = 0;\n+\tint i;\n\n \t/* This checks and barfs if author is badly specified */\n \tdetermine_author_info(author_ident);\n@@ -803,6 +805,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \tif (clean_message_contents)\n \t\tstrbuf_stripspace(&sb, 0);\n\n+\tfor (i = 0; i < coauthors.nr; i++) {\n+\t\tappend_coauthor(&sb, coauthors.items[i].string);\n+\t}\n+\n \tif (signoff)\n \t\tappend_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);\n\n@@ -1504,6 +1510,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"squash\", &squash_message, N_(\"commit\"), N_(\"use autosquash formatted message to squash specified commit\")),\n \t\tOPT_BOOL(0, \"reset-author\", &renew_authorship, N_(\"the commit is authored by me now (used with -C/-c/--amend)\")),\n \t\tOPT_BOOL('s', \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n+\t\tOPT_STRING_LIST(0, \"coauthor\", &coauthors, N_(\"co-author\"), N_(\"add Co-authored-by:\")),\n \t\tOPT_FILENAME('t', \"template\", &template_file, N_(\"use specified template file\")),\n \t\tOPT_BOOL('e', \"edit\", &edit_flag, N_(\"force edit of commit\")),\n \t\tOPT_CLEANUP(&cleanup_arg),\ndiff --git a/sequencer.c b/sequencer.c\nindex d648aaf416..8958a22470 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -36,6 +36,7 @@\n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n\n static const char sign_off_header[] = \"Signed-off-by: \";\n+static const char coauthor_header[] = \"Co-authored-by: \";\n static const char cherry_picked_prefix[] = \"(cherry picked from commit \";\n\n GIT_PATH_FUNC(git_path_commit_editmsg, \"COMMIT_EDITMSG\")\n@@ -4385,15 +4386,9 @@ int sequencer_pick_revisions(struct repository *r,\n \treturn res;\n }\n\n-void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)\n+static void append_footer(struct strbuf *msgbuf, struct strbuf* sob, size_t ignore_footer, size_t no_dup_sob)\n {\n-\tunsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;\n-\tstruct strbuf sob = STRBUF_INIT;\n-\tint has_footer;\n-\n-\tstrbuf_addstr(&sob, sign_off_header);\n-\tstrbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT));\n-\tstrbuf_addch(&sob, '\\n');\n+\tsize_t has_footer;\n\n \tif (!ignore_footer)\n \t\tstrbuf_complete_line(msgbuf);\n@@ -4402,11 +4397,11 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)\n \t * If the whole message buffer is equal to the sob, pretend that we\n \t * found a conforming footer with a matching sob\n \t */\n-\tif (msgbuf->len - ignore_footer == sob.len &&\n-\t    !strncmp(msgbuf->buf, sob.buf, sob.len))\n+\tif (msgbuf->len - ignore_footer == sob->len &&\n+\t    !strncmp(msgbuf->buf, sob->buf, sob->len))\n \t\thas_footer = 3;\n \telse\n-\t\thas_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);\n+\t\thas_footer = has_conforming_footer(msgbuf, sob, ignore_footer);\n\n \tif (!has_footer) {\n \t\tconst char *append_newlines = NULL;\n@@ -4440,7 +4435,32 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)\n\n \tif (has_footer != 3 && (!no_dup_sob || has_footer != 2))\n \t\tstrbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,\n-\t\t\t\tsob.buf, sob.len);\n+\t\t\t\tsob->buf, sob->len);\n+}\n+\n+void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)\n+{\n+\tunsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;\n+\tstruct strbuf sob = STRBUF_INIT;\n+\n+\tstrbuf_addstr(&sob, sign_off_header);\n+\tstrbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT));\n+\tstrbuf_addch(&sob, '\\n');\n+\n+\tappend_footer(msgbuf, &sob, ignore_footer, no_dup_sob);\n+\n+\tstrbuf_release(&sob);\n+}\n+\n+void append_coauthor(struct strbuf *msgbuf, const char *coauthor)\n+{\n+\tstruct strbuf sob = STRBUF_INIT;\n+\n+\tstrbuf_addstr(&sob, coauthor_header);\n+\tstrbuf_addstr(&sob, coauthor);\n+\tstrbuf_addch(&sob, '\\n');\n+\n+\tappend_footer(msgbuf, &sob, 0, 1);\n\n \tstrbuf_release(&sob);\n }\ndiff --git a/sequencer.h b/sequencer.h\nindex 574260f621..e36489fce7 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -170,6 +170,8 @@ int todo_list_rearrange_squash(struct todo_list *todo_list);\n  */\n void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag);\n\n+void append_coauthor(struct strbuf *msgbuf, const char* co_author);\n+\n void append_conflicts_hint(struct index_state *istate,\n \t\tstruct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode);\n enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\ndiff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh\nindex 14c92e4c25..5ed6735cf4 100755\n--- a/t/t7502-commit-porcelain.sh\n+++ b/t/t7502-commit-porcelain.sh\n@@ -138,6 +138,17 @@ test_expect_success 'partial removal' '\n\n '\n\n+test_expect_success 'co-author' '\n+\n+\t>coauthor &&\n+\tgit add coauthor &&\n+\tgit commit -m \"thank you\" --co-author=\"Author <author@example.com>\" &&\n+\tgit cat-file commit HEAD >commit.msg &&\n+\tsed -ne \"s/Co-authored-by: //p\" commit.msg >actual &&\n+\techo \"Author <author@example.com>\" >expected &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'sign off' '\n\n \t>positive &&\n--\n2.22.0.rc3\n"},{"id":"383663","messageId":"20191008083537.GA6727@generichostname","threadId":"51993","inReplyTo":"20191008074935.10972-1-toon@iotcl.com","subject":"Re: [PATCH 1/1] commit: add support to provide --coauthor","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-10-08T08:35:37Z","receivedAt":"2019-10-08T08:35:44Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Welcome to the Git community, Toon!\n\nWow, I never realised that people actually read my braindump of issues\non GGG. Thanks for taking this on.\n\nFirst some housekeeping, when formatting your patches, it's a good idea\nto use `git format-patch --thread` so that your patches are grouped\ntogether by thread.\n\nOn Tue, Oct 08, 2019 at 09:49:35AM +0200, Toon Claes wrote:\n> Add support to provide the Co-author when committing. For each\n> co-author provided with --coauthor=<coauthor>, a line is added at the\n\nI know you're taking the name from my GGG issue but perhaps --co-author\nwould look better? \n\n> bottom of the commit message, like this:\n> \n>     Co-authored-by: <coauthor>\n> \n> It's a common practice use when pairing up with other people and both\n> authors want to in the commit message.\n> \n> Co-authored-by: Zeger-Jan van de Weg <git@zjvandeweg.nl>\n\nNice ;)\n\n> Signed-off-by: Toon Claes <toon@iotcl.com>\n> ---\n>  Documentation/git-commit.txt |  5 ++++\n>  builtin/commit.c             |  7 ++++++\n>  sequencer.c                  | 44 ++++++++++++++++++++++++++----------\n>  sequencer.h                  |  2 ++\n>  t/t7502-commit-porcelain.sh  | 11 +++++++++\n>  5 files changed, 57 insertions(+), 12 deletions(-)\n> \n> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n> index afa7b75a23..c059944e38 100644\n> --- a/Documentation/git-commit.txt\n> +++ b/Documentation/git-commit.txt\n> @@ -140,6 +140,11 @@ OPTIONS\n>  \tcommit by that author (i.e. rev-list --all -i --author=<author>);\n>  \tthe commit author is then copied from the first such commit found.\n> \n> +--coauthor=<coauthor>::\n> +        Add a Co-authored-by line with the specified author. Specify the\n\nThis line is indented with spaces but it should be changed to a single\ntab.\n\n> +\tauthor using the standard `Co Artur <co-artur@example.com>`\n> +\tformat.\n> +\n>  --date=<date>::\n>  \tOverride the author date used in the commit.\n> \n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index ae7aaf6dc6..feb423ed6f 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -110,6 +110,7 @@ static int config_commit_verbose = -1; /* unspecified */\n>  static int no_post_rewrite, allow_empty_message;\n>  static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n>  static char *sign_commit;\n> +static struct string_list coauthors = STRING_LIST_INIT_NODUP;\n> \n>  /*\n>   * The default commit message cleanup mode will remove the lines\n> @@ -672,6 +673,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>  \tint clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE);\n>  \tint old_display_comment_prefix;\n>  \tint merge_contains_scissors = 0;\n> +\tint i;\n> \n>  \t/* This checks and barfs if author is badly specified */\n>  \tdetermine_author_info(author_ident);\n> @@ -803,6 +805,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>  \tif (clean_message_contents)\n>  \t\tstrbuf_stripspace(&sb, 0);\n> \n> +\tfor (i = 0; i < coauthors.nr; i++) {\n> +\t\tappend_coauthor(&sb, coauthors.items[i].string);\n> +\t}\n> +\n>  \tif (signoff)\n>  \t\tappend_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);\n> \n> @@ -1504,6 +1510,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>  \t\tOPT_STRING(0, \"squash\", &squash_message, N_(\"commit\"), N_(\"use autosquash formatted message to squash specified commit\")),\n>  \t\tOPT_BOOL(0, \"reset-author\", &renew_authorship, N_(\"the commit is authored by me now (used with -C/-c/--amend)\")),\n>  \t\tOPT_BOOL('s', \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n> +\t\tOPT_STRING_LIST(0, \"coauthor\", &coauthors, N_(\"co-author\"), N_(\"add Co-authored-by:\")),\n>  \t\tOPT_FILENAME('t', \"template\", &template_file, N_(\"use specified template file\")),\n>  \t\tOPT_BOOL('e', \"edit\", &edit_flag, N_(\"force edit of commit\")),\n>  \t\tOPT_CLEANUP(&cleanup_arg),\n> diff --git a/sequencer.c b/sequencer.c\n> index d648aaf416..8958a22470 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -36,6 +36,7 @@\n>  #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n> \n>  static const char sign_off_header[] = \"Signed-off-by: \";\n> +static const char coauthor_header[] = \"Co-authored-by: \";\n>  static const char cherry_picked_prefix[] = \"(cherry picked from commit \";\n> \n>  GIT_PATH_FUNC(git_path_commit_editmsg, \"COMMIT_EDITMSG\")\n> @@ -4385,15 +4386,9 @@ int sequencer_pick_revisions(struct repository *r,\n>  \treturn res;\n>  }\n> \n> -void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)\n> +static void append_footer(struct strbuf *msgbuf, struct strbuf* sob, size_t ignore_footer, size_t no_dup_sob)\n\nThe * for pointers should go with the name, not the type. Also, `sob` is\na misleading name since it means \"Signed-off-by\". In this case, we're\nusing it as a general footer (since it can include Co-author lines too)\nso perhaps rename this to `footer`? \n\nFinally, as a nit, can we mark sob as const?\n\n>  {\n> -\tunsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;\n> -\tstruct strbuf sob = STRBUF_INIT;\n> -\tint has_footer;\n> -\n> -\tstrbuf_addstr(&sob, sign_off_header);\n> -\tstrbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT));\n> -\tstrbuf_addch(&sob, '\\n');\n> +\tsize_t has_footer;\n\nWhy was this changed into a size_t? has_conforming_footer() below\nreturns int so we should leave it as is.\n\n> \n>  \tif (!ignore_footer)\n>  \t\tstrbuf_complete_line(msgbuf);\n> @@ -4402,11 +4397,11 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)\n>  \t * If the whole message buffer is equal to the sob, pretend that we\n>  \t * found a conforming footer with a matching sob\n>  \t */\n> -\tif (msgbuf->len - ignore_footer == sob.len &&\n> -\t    !strncmp(msgbuf->buf, sob.buf, sob.len))\n> +\tif (msgbuf->len - ignore_footer == sob->len &&\n> +\t    !strncmp(msgbuf->buf, sob->buf, sob->len))\n>  \t\thas_footer = 3;\n>  \telse\n> -\t\thas_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);\n> +\t\thas_footer = has_conforming_footer(msgbuf, sob, ignore_footer);\n\nSince you're touching this area, could you please a prepatory patch\nbefore this one that changes the function signature:\n\n\t- static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, size_t ignore_footer)                                                          \n\t+ static int has_conforming_footer(struct strbuf *sb, const struct strbuf *footer, size_t ignore_footer)                                                          \n\nThat way, you'll be able to mark `sob` const in append_footer() and\nalso, `sob` can be renamed to `footer` which should be less misleading.\n\n> \n>  \tif (!has_footer) {\n>  \t\tconst char *append_newlines = NULL;\n> @@ -4440,7 +4435,32 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)\n> \n>  \tif (has_footer != 3 && (!no_dup_sob || has_footer != 2))\n>  \t\tstrbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,\n> -\t\t\t\tsob.buf, sob.len);\n> +\t\t\t\tsob->buf, sob->len);\n> +}\n> +\n> +void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)\n> +{\n> +\tunsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;\n> +\tstruct strbuf sob = STRBUF_INIT;\n> +\n> +\tstrbuf_addstr(&sob, sign_off_header);\n> +\tstrbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT));\n> +\tstrbuf_addch(&sob, '\\n');\n> +\n> +\tappend_footer(msgbuf, &sob, ignore_footer, no_dup_sob);\n> +\n> +\tstrbuf_release(&sob);\n> +}\n> +\n> +void append_coauthor(struct strbuf *msgbuf, const char *coauthor)\n> +{\n> +\tstruct strbuf sob = STRBUF_INIT;\n\nSame, this `sob` should definitely be written as `footer` or maybe\n`coauthor`.\n> +\n> +\tstrbuf_addstr(&sob, coauthor_header);\n> +\tstrbuf_addstr(&sob, coauthor);\n> +\tstrbuf_addch(&sob, '\\n');\n> +\n> +\tappend_footer(msgbuf, &sob, 0, 1);\n> \n>  \tstrbuf_release(&sob);\n>  }\n> diff --git a/sequencer.h b/sequencer.h\n> index 574260f621..e36489fce7 100644\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -170,6 +170,8 @@ int todo_list_rearrange_squash(struct todo_list *todo_list);\n>   */\n>  void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag);\n> \n> +void append_coauthor(struct strbuf *msgbuf, const char* co_author);\n\nAsterisk should also be attached to the name.\n\n> +\n>  void append_conflicts_hint(struct index_state *istate,\n>  \t\tstruct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode);\n>  enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n> diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh\n> index 14c92e4c25..5ed6735cf4 100755\n> --- a/t/t7502-commit-porcelain.sh\n> +++ b/t/t7502-commit-porcelain.sh\n> @@ -138,6 +138,17 @@ test_expect_success 'partial removal' '\n> \n>  '\n> \n> +test_expect_success 'co-author' '\n> +\n> +\t>coauthor &&\n> +\tgit add coauthor &&\n> +\tgit commit -m \"thank you\" --co-author=\"Author <author@example.com>\" &&\n> +\tgit cat-file commit HEAD >commit.msg &&\n> +\tsed -ne \"s/Co-authored-by: //p\" commit.msg >actual &&\n> +\techo \"Author <author@example.com>\" >expected &&\n> +\ttest_cmp expected actual\n> +'\n> +\n\nThis test looks good to me.\n\nThanks again for taking this on,\n\nDenton\n\n>  test_expect_success 'sign off' '\n> \n>  \t>positive &&\n> --\n> 2.22.0.rc3\n"},{"id":"383668","messageId":"76753409-34f2-d016-81a9-ea6e3d0e34bd@gmail.com","threadId":"51993","inReplyTo":"20191008074935.10972-1-toon@iotcl.com","subject":"Re: [PATCH 1/1] commit: add support to provide --coauthor","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-10-08T10:11:45Z","receivedAt":"2019-10-08T10:11:51Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Toon & Zeger-Jan\n\nOn 08/10/2019 08:49, Toon Claes wrote:\n> Add support to provide the Co-author when committing. For each\n> co-author provided with --coauthor=<coauthor>, a line is added at the\n> bottom of the commit message, like this:\n> \n>      Co-authored-by: <coauthor>\n> \n> It's a common practice use when pairing up with other people and both\n> authors want to in the commit message.\n\nThanks for the patch, it's looking good. I can see this being useful to \nsome people - I like the way the patch itself is co-authored.\n\n> \n> Co-authored-by: Zeger-Jan van de Weg <git@zjvandeweg.nl>\n> Signed-off-by: Toon Claes <toon@iotcl.com>\n> ---\n>   Documentation/git-commit.txt |  5 ++++\n>   builtin/commit.c             |  7 ++++++\n>   sequencer.c                  | 44 ++++++++++++++++++++++++++----------\n>   sequencer.h                  |  2 ++\n>   t/t7502-commit-porcelain.sh  | 11 +++++++++\n>   5 files changed, 57 insertions(+), 12 deletions(-)\n> \n> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n> index afa7b75a23..c059944e38 100644\n> --- a/Documentation/git-commit.txt\n> +++ b/Documentation/git-commit.txt\n> @@ -140,6 +140,11 @@ OPTIONS\n>   \tcommit by that author (i.e. rev-list --all -i --author=<author>);\n>   \tthe commit author is then copied from the first such commit found.\n> \n> +--coauthor=<coauthor>::\n> +        Add a Co-authored-by line with the specified author. Specify the\n> +\tauthor using the standard `Co Artur <co-artur@example.com>`\n> +\tformat.\n> +\n>   --date=<date>::\n>   \tOverride the author date used in the commit.\n> \n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index ae7aaf6dc6..feb423ed6f 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -110,6 +110,7 @@ static int config_commit_verbose = -1; /* unspecified */\n>   static int no_post_rewrite, allow_empty_message;\n>   static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n>   static char *sign_commit;\n> +static struct string_list coauthors = STRING_LIST_INIT_NODUP;\n> \n>   /*\n>    * The default commit message cleanup mode will remove the lines\n> @@ -672,6 +673,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>   \tint clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE);\n>   \tint old_display_comment_prefix;\n>   \tint merge_contains_scissors = 0;\n> +\tint i;\n> \n>   \t/* This checks and barfs if author is badly specified */\n>   \tdetermine_author_info(author_ident);\n> @@ -803,6 +805,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>   \tif (clean_message_contents)\n>   \t\tstrbuf_stripspace(&sb, 0);\n> \n> +\tfor (i = 0; i < coauthors.nr; i++) {\n> +\t\tappend_coauthor(&sb, coauthors.items[i].string);\n\nIf you look at the existing code that's calling append_signoff() it does\n   append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0)\nThe purpose of ignore_non_trailer is to ignore comment and conflicts \nmessages at the end of the commit message (there's more detail in a \ncomment above it's definition in builtin/commit.c). I think we need to \npass this offset when adding co-authors as well.\n\nOne question - what is the desired de-duplication behavior of \nCo-authored-by:? What happens if there is already a matching \nCo-authored-by: footer? (It is also possible for the trailers code to \nonly ignore an existing footer if it's the last one.) What happens if \nthe same Co-authored-by: is duplicated on the command line? It would be \nnice to have this defined and tests to check it's enforced.\n\nAnother useful addition would be to check that the footer value looks \nsane but that could come later - I don't think we do that for any other \nfooters at the moment (though I haven't checked to see if that's really \ntrue)\n\n> +\t}\n> +\n>   \tif (signoff)\n>   \t\tappend_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);\n> \n> @@ -1504,6 +1510,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>   \t\tOPT_STRING(0, \"squash\", &squash_message, N_(\"commit\"), N_(\"use autosquash formatted message to squash specified commit\")),\n>   \t\tOPT_BOOL(0, \"reset-author\", &renew_authorship, N_(\"the commit is authored by me now (used with -C/-c/--amend)\")),\n>   \t\tOPT_BOOL('s', \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n> +\t\tOPT_STRING_LIST(0, \"coauthor\", &coauthors, N_(\"co-author\"), N_(\"add Co-authored-by:\")),\n>   \t\tOPT_FILENAME('t', \"template\", &template_file, N_(\"use specified template file\")),\n>   \t\tOPT_BOOL('e', \"edit\", &edit_flag, N_(\"force edit of commit\")),\n>   \t\tOPT_CLEANUP(&cleanup_arg),\n> diff --git a/sequencer.c b/sequencer.c\n> index d648aaf416..8958a22470 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -36,6 +36,7 @@\n>   #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n> \n>   static const char sign_off_header[] = \"Signed-off-by: \";\n> +static const char coauthor_header[] = \"Co-authored-by: \";\n>   static const char cherry_picked_prefix[] = \"(cherry picked from commit \";\n> \n>   GIT_PATH_FUNC(git_path_commit_editmsg, \"COMMIT_EDITMSG\")\n> @@ -4385,15 +4386,9 @@ int sequencer_pick_revisions(struct repository *r,\n>   \treturn res;\n>   }\n> \n> -void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)\n> +static void append_footer(struct strbuf *msgbuf, struct strbuf* sob, size_t ignore_footer, size_t no_dup_sob)\n>   {\n> -\tunsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;\n> -\tstruct strbuf sob = STRBUF_INIT;\n> -\tint has_footer;\n> -\n> -\tstrbuf_addstr(&sob, sign_off_header);\n> -\tstrbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT));\n> -\tstrbuf_addch(&sob, '\\n');\n> +\tsize_t has_footer;\n> \n>   \tif (!ignore_footer)\n>   \t\tstrbuf_complete_line(msgbuf);\n> @@ -4402,11 +4397,11 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)\n>   \t * If the whole message buffer is equal to the sob, pretend that we\n>   \t * found a conforming footer with a matching sob\n>   \t */\n> -\tif (msgbuf->len - ignore_footer == sob.len &&\n> -\t    !strncmp(msgbuf->buf, sob.buf, sob.len))\n> +\tif (msgbuf->len - ignore_footer == sob->len &&\n> +\t    !strncmp(msgbuf->buf, sob->buf, sob->len))\n>   \t\thas_footer = 3;\n>   \telse\n> -\t\thas_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);\n> +\t\thas_footer = has_conforming_footer(msgbuf, sob, ignore_footer);\n> \n>   \tif (!has_footer) {\n>   \t\tconst char *append_newlines = NULL;\n> @@ -4440,7 +4435,32 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)\n> \n>   \tif (has_footer != 3 && (!no_dup_sob || has_footer != 2))\n>   \t\tstrbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,\n> -\t\t\t\tsob.buf, sob.len);\n> +\t\t\t\tsob->buf, sob->len);\n> +}\n> +\n> +void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)\n> +{\n> +\tunsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;\n> +\tstruct strbuf sob = STRBUF_INIT;\n> +\n> +\tstrbuf_addstr(&sob, sign_off_header);\n> +\tstrbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT));\n> +\tstrbuf_addch(&sob, '\\n');\n> +\n> +\tappend_footer(msgbuf, &sob, ignore_footer, no_dup_sob);\n> +\n> +\tstrbuf_release(&sob);\n> +}\n> +\n> +void append_coauthor(struct strbuf *msgbuf, const char *coauthor)\n> +{\n> +\tstruct strbuf sob = STRBUF_INIT;\n> +\n> +\tstrbuf_addstr(&sob, coauthor_header);\n> +\tstrbuf_addstr(&sob, coauthor);\n> +\tstrbuf_addch(&sob, '\\n');\n> +\n> +\tappend_footer(msgbuf, &sob, 0, 1);\n\nAs we have a constant for APPEND_SIGNOFF_DEDUP can we use it here please \nrather than '1' which does not covey the same meaning to the author. \nAlso as I said above I think you want to pass in a real value for \nignore_footer not zero\n\n> \n>   \tstrbuf_release(&sob);\n>   }\n> diff --git a/sequencer.h b/sequencer.h\n> index 574260f621..e36489fce7 100644\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -170,6 +170,8 @@ int todo_list_rearrange_squash(struct todo_list *todo_list);\n>    */\n>   void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag);\n> \n> +void append_coauthor(struct strbuf *msgbuf, const char* co_author);\n> +\n>   void append_conflicts_hint(struct index_state *istate,\n>   \t\tstruct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode);\n>   enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n> diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh\n> index 14c92e4c25..5ed6735cf4 100755\n> --- a/t/t7502-commit-porcelain.sh\n> +++ b/t/t7502-commit-porcelain.sh\n> @@ -138,6 +138,17 @@ test_expect_success 'partial removal' '\n> \n>   '\n> \n> +test_expect_success 'co-author' '\n> +\n> +\t>coauthor &&\n> +\tgit add coauthor &&\n> +\tgit commit -m \"thank you\" --co-author=\"Author <author@example.com>\" &&\n> +\tgit cat-file commit HEAD >commit.msg &&\n> +\tsed -ne \"s/Co-authored-by: //p\" commit.msg >actual &&\n> +\techo \"Author <author@example.com>\" >expected &&\n> +\ttest_cmp expected actual\n> +'\n\nThis is fine as far as it goes but it would be nice to test the \nde-duplication behavior once that's defined\n\nBest Wishes\n\nPhillip\n\n>   test_expect_success 'sign off' '\n> \n>   \t>positive &&\n> --\n> 2.22.0.rc3\n> \n"},{"id":"383673","messageId":"8a018227-b055-15e5-d1c1-ac41f24c155b@gmail.com","threadId":"51993","inReplyTo":"76753409-34f2-d016-81a9-ea6e3d0e34bd@gmail.com","subject":"Re: [PATCH 1/1] commit: add support to provide --coauthor","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-10-08T12:04:15Z","receivedAt":"2019-10-08T12:04:21Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 08/10/2019 11:11, Phillip Wood wrote:\n> Hi Toon & Zeger-Jan\n> \n> On 08/10/2019 08:49, Toon Claes wrote:\n>> Add support to provide the Co-author when committing. For each\n>> co-author provided with --coauthor=<coauthor>, a line is added at the\n>> bottom of the commit message, like this:\n>>\n>>      Co-authored-by: <coauthor>\n>>\n>> It's a common practice use when pairing up with other people and both\n>> authors want to in the commit message.\n> \n> Thanks for the patch, it's looking good. I can see this being useful to\n> some people - I like the way the patch itself is co-authored.\n> [...]\n>> @@ -803,6 +805,10 @@ static int prepare_to_commit(const char\n>> *index_file, const char *prefix,\n>>       if (clean_message_contents)\n>>           strbuf_stripspace(&sb, 0);\n>>\n>> +    for (i = 0; i < coauthors.nr; i++) {\n>> +        append_coauthor(&sb, coauthors.items[i].string);\n> \n> If you look at the existing code that's calling append_signoff() it does\n>   append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0)\n> The purpose of ignore_non_trailer is to ignore comment and conflicts\n> messages at the end of the commit message (there's more detail in a\n> comment above it's definition in builtin/commit.c). I think we need to\n> pass this offset when adding co-authors as well.\n> \n> One question - what is the desired de-duplication behavior of\n> Co-authored-by:? What happens if there is already a matching\n> Co-authored-by: footer? (It is also possible for the trailers code to\n> only ignore an existing footer if it's the last one.) What happens if\n> the same Co-authored-by: is duplicated on the command line? It would be\n> nice to have this defined and tests to check it's enforced.\n\nI should give a bit more detail here. git-interpret-trailers gives more\ncontrol over handling of duplicates that is configurable via 'git\nconfig' than 'commit --signoff' does. The reason for this is that\n'commit --signoff' predates the interpret-trailers stuff. As we're\nadding a new footer command we should decide if we want it to act like\n--signoff or give the user the ability to configure the de-duplication\nbehavior by using the interpret-trailers code path instead. (I think\nformat-patch --signoff respects the interpret-trailers config variables\nbut 'am --signoff' and 'cherry-pick --signoff' do not.\n\n> \n> Another useful addition would be to check that the footer value looks\n> sane but that could come later\n\nLooking at the way commit handles --author (grep for force_author in\nbuiltin/commit.c) it should be simple to add these checks - just call\nsplit_ident() and check it returns zero. --author also checks if the\nstring contains '<' and if it doesn't it uses the string given on the\ncommand line to lookup a matching author in the commit log - that could\nbe a nice feature to use here too (see the code that calls\nfind_author_by_nickname()).\n\nBest Wishes\n\nPhillip\n\n - I don't think we do that for any other\n> footers at the moment (though I haven't checked to see if that's really\n> true)\n> \n>> +    }\n>> +\n>>       if (signoff)\n>>           append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);\n>>\n>> @@ -1504,6 +1510,7 @@ int cmd_commit(int argc, const char **argv,\n>> const char *prefix)\n>>           OPT_STRING(0, \"squash\", &squash_message, N_(\"commit\"),\n>> N_(\"use autosquash formatted message to squash specified commit\")),\n>>           OPT_BOOL(0, \"reset-author\", &renew_authorship, N_(\"the\n>> commit is authored by me now (used with -C/-c/--amend)\")),\n>>           OPT_BOOL('s', \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n>> +        OPT_STRING_LIST(0, \"coauthor\", &coauthors, N_(\"co-author\"),\n>> N_(\"add Co-authored-by:\")),\n>>           OPT_FILENAME('t', \"template\", &template_file, N_(\"use\n>> specified template file\")),\n>>           OPT_BOOL('e', \"edit\", &edit_flag, N_(\"force edit of commit\")),\n>>           OPT_CLEANUP(&cleanup_arg),\n>> diff --git a/sequencer.c b/sequencer.c\n>> index d648aaf416..8958a22470 100644\n>> --- a/sequencer.c\n>> +++ b/sequencer.c\n>> @@ -36,6 +36,7 @@\n>>   #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n>>\n>>   static const char sign_off_header[] = \"Signed-off-by: \";\n>> +static const char coauthor_header[] = \"Co-authored-by: \";\n>>   static const char cherry_picked_prefix[] = \"(cherry picked from\n>> commit \";\n>>\n>>   GIT_PATH_FUNC(git_path_commit_editmsg, \"COMMIT_EDITMSG\")\n>> @@ -4385,15 +4386,9 @@ int sequencer_pick_revisions(struct repository *r,\n>>       return res;\n>>   }\n>>\n>> -void append_signoff(struct strbuf *msgbuf, size_t ignore_footer,\n>> unsigned flag)\n>> +static void append_footer(struct strbuf *msgbuf, struct strbuf* sob,\n>> size_t ignore_footer, size_t no_dup_sob)\n>>   {\n>> -    unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;\n>> -    struct strbuf sob = STRBUF_INIT;\n>> -    int has_footer;\n>> -\n>> -    strbuf_addstr(&sob, sign_off_header);\n>> -    strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT));\n>> -    strbuf_addch(&sob, '\\n');\n>> +    size_t has_footer;\n>>\n>>       if (!ignore_footer)\n>>           strbuf_complete_line(msgbuf);\n>> @@ -4402,11 +4397,11 @@ void append_signoff(struct strbuf *msgbuf,\n>> size_t ignore_footer, unsigned flag)\n>>        * If the whole message buffer is equal to the sob, pretend that we\n>>        * found a conforming footer with a matching sob\n>>        */\n>> -    if (msgbuf->len - ignore_footer == sob.len &&\n>> -        !strncmp(msgbuf->buf, sob.buf, sob.len))\n>> +    if (msgbuf->len - ignore_footer == sob->len &&\n>> +        !strncmp(msgbuf->buf, sob->buf, sob->len))\n>>           has_footer = 3;\n>>       else\n>> -        has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer);\n>> +        has_footer = has_conforming_footer(msgbuf, sob, ignore_footer);\n>>\n>>       if (!has_footer) {\n>>           const char *append_newlines = NULL;\n>> @@ -4440,7 +4435,32 @@ void append_signoff(struct strbuf *msgbuf,\n>> size_t ignore_footer, unsigned flag)\n>>\n>>       if (has_footer != 3 && (!no_dup_sob || has_footer != 2))\n>>           strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,\n>> -                sob.buf, sob.len);\n>> +                sob->buf, sob->len);\n>> +}\n>> +\n>> +void append_signoff(struct strbuf *msgbuf, size_t ignore_footer,\n>> unsigned flag)\n>> +{\n>> +    unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;\n>> +    struct strbuf sob = STRBUF_INIT;\n>> +\n>> +    strbuf_addstr(&sob, sign_off_header);\n>> +    strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT));\n>> +    strbuf_addch(&sob, '\\n');\n>> +\n>> +    append_footer(msgbuf, &sob, ignore_footer, no_dup_sob);\n>> +\n>> +    strbuf_release(&sob);\n>> +}\n>> +\n>> +void append_coauthor(struct strbuf *msgbuf, const char *coauthor)\n>> +{\n>> +    struct strbuf sob = STRBUF_INIT;\n>> +\n>> +    strbuf_addstr(&sob, coauthor_header);\n>> +    strbuf_addstr(&sob, coauthor);\n>> +    strbuf_addch(&sob, '\\n');\n>> +\n>> +    append_footer(msgbuf, &sob, 0, 1);\n> \n> As we have a constant for APPEND_SIGNOFF_DEDUP can we use it here please\n> rather than '1' which does not covey the same meaning to the author.\n> Also as I said above I think you want to pass in a real value for\n> ignore_footer not zero\n> \n>>\n>>       strbuf_release(&sob);\n>>   }\n>> diff --git a/sequencer.h b/sequencer.h\n>> index 574260f621..e36489fce7 100644\n>> --- a/sequencer.h\n>> +++ b/sequencer.h\n>> @@ -170,6 +170,8 @@ int todo_list_rearrange_squash(struct todo_list\n>> *todo_list);\n>>    */\n>>   void append_signoff(struct strbuf *msgbuf, size_t ignore_footer,\n>> unsigned flag);\n>>\n>> +void append_coauthor(struct strbuf *msgbuf, const char* co_author);\n>> +\n>>   void append_conflicts_hint(struct index_state *istate,\n>>           struct strbuf *msgbuf, enum commit_msg_cleanup_mode\n>> cleanup_mode);\n>>   enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,\n>> diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh\n>> index 14c92e4c25..5ed6735cf4 100755\n>> --- a/t/t7502-commit-porcelain.sh\n>> +++ b/t/t7502-commit-porcelain.sh\n>> @@ -138,6 +138,17 @@ test_expect_success 'partial removal' '\n>>\n>>   '\n>>\n>> +test_expect_success 'co-author' '\n>> +\n>> +    >coauthor &&\n>> +    git add coauthor &&\n>> +    git commit -m \"thank you\" --co-author=\"Author\n>> <author@example.com>\" &&\n>> +    git cat-file commit HEAD >commit.msg &&\n>> +    sed -ne \"s/Co-authored-by: //p\" commit.msg >actual &&\n>> +    echo \"Author <author@example.com>\" >expected &&\n>> +    test_cmp expected actual\n>> +'\n> \n> This is fine as far as it goes but it would be nice to test the\n> de-duplication behavior once that's defined\n> \n> Best Wishes\n> \n> Phillip\n> \n>>   test_expect_success 'sign off' '\n>>\n>>       >positive &&\n>> -- \n>> 2.22.0.rc3\n>>\n\n"},{"id":"383722","messageId":"20191009014039.GA10802@szeder.dev","threadId":"51993","inReplyTo":"20191008074935.10972-1-toon@iotcl.com","subject":"Re: [PATCH 1/1] commit: add support to provide --coauthor","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-09T01:40:39Z","receivedAt":"2019-10-09T01:40:48Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Oct 08, 2019 at 09:49:35AM +0200, Toon Claes wrote:\n> Add support to provide the Co-author when committing. For each\n> co-author provided with --coauthor=<coauthor>, a line is added at the\n> bottom of the commit message, like this:\n> \n>     Co-authored-by: <coauthor>\n> \n> It's a common practice use when pairing up with other people and both\n> authors want to in the commit message.\n\nI wonder how we are supposed to use this trailer in the Git project,\nin particular in combination with Signed-off-by.  Should all\n(co)authors sign off as well?  Or will Co-authored-by imply\nSigned-off-by?\n\n"},{"id":"383726","messageId":"xmqqr23mlkxo.fsf@gitster-ct.c.googlers.com","threadId":"51993","inReplyTo":"20191009014039.GA10802@szeder.dev","subject":"Re: [PATCH 1/1] commit: add support to provide --coauthor","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-09T02:19:47Z","receivedAt":"2019-10-09T02:19:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> On Tue, Oct 08, 2019 at 09:49:35AM +0200, Toon Claes wrote:\n>> Add support to provide the Co-author when committing. For each\n>> co-author provided with --coauthor=<coauthor>, a line is added at the\n>> bottom of the commit message, like this:\n>> \n>>     Co-authored-by: <coauthor>\n>> \n>> It's a common practice use when pairing up with other people and both\n>> authors want to in the commit message.\n>\n> I wonder how we are supposed to use this trailer in the Git project,\n> in particular in combination with Signed-off-by.  Should all\n> (co)authors sign off as well?  Or will Co-authored-by imply\n> Signed-off-by?\n\nI think we have been happy with (1) a comment at the end of the log\nmessage that says X worked together with Y and Z to produce this\npatch, and (2) the trailer block that has S-o-b: from X, Y and Z,\nwithout any need for Co-authored-by: trailer so far, and I do not\nsee any reason to change it in this project.\n\nIf other projects wants to use such a footer, that's their choice,\nbut I am fairly negative to the idea to open the gate to unbounded\nnumber of new options for new trailer lines.  We do not even have\nsuch options like --acked=<acker>, --reported=<reporter>, for the\ntrailers that are actively used already (and to make sure nobody\nmisunderstands, I do not think it is a good idea to add such\nindividual options).\n\nThanks.\n\n"},{"id":"383742","messageId":"20191009112021.h2ywofk2pivdyavd@camp.crustytoothpaste.net","threadId":"51993","inReplyTo":"xmqqr23mlkxo.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] commit: add support to provide --coauthor","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-10-09T11:20:21Z","receivedAt":"2019-10-09T11:20:29Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-10-09 at 02:19:47, Junio C Hamano wrote:\n> I think we have been happy with (1) a comment at the end of the log\n> message that says X worked together with Y and Z to produce this\n> patch, and (2) the trailer block that has S-o-b: from X, Y and Z,\n> without any need for Co-authored-by: trailer so far, and I do not\n> see any reason to change it in this project.\n> \n> If other projects wants to use such a footer, that's their choice,\n> but I am fairly negative to the idea to open the gate to unbounded\n> number of new options for new trailer lines.  We do not even have\n> such options like --acked=<acker>, --reported=<reporter>, for the\n> trailers that are actively used already (and to make sure nobody\n> misunderstands, I do not think it is a good idea to add such\n> individual options).\n\nIf we don't want to add this option explicitly, which I understand, it\nmay be useful to add an option to add arbitrary commit trailers to a\ncommit message when creating it.\n\nGit uses a patch-based workflow, where adding trailers is relatively\neasy, but for folks using pull request-based workflows without signoffs,\nusing Co-authored-by can be a great way to note when folks have worked\ntogether on a patch.  An option to git commit such as --trailer would\nallow folks to add whatever trailers they wish, including this one,\nwithout us needing to bless particular options.\n\nI, for one, would be excited to see a series which did this.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"383743","messageId":"xmqqimoykv49.fsf@gitster-ct.c.googlers.com","threadId":"51993","inReplyTo":"20191009112021.h2ywofk2pivdyavd@camp.crustytoothpaste.net","subject":"Re: [PATCH 1/1] commit: add support to provide --coauthor","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-09T11:37:26Z","receivedAt":"2019-10-09T11:37:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> ...  An option to git commit such as --trailer would\n> allow folks to add whatever trailers they wish, including this one,\n> without us needing to bless particular options.\n\nYes, that was what I was hoping to become the \"core\" of the idea,\nwith possibly syntax sugar to make it easier to use.\n\nThanks for filling in what I left unsaid ;-)\n\n"},{"id":"383760","messageId":"20191009203105.GA7007@sigill.intra.peff.net","threadId":"51993","inReplyTo":"xmqqr23mlkxo.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] commit: add support to provide --coauthor","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-10-09T20:31:06Z","receivedAt":"2019-10-09T20:31:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 09, 2019 at 11:19:47AM +0900, Junio C Hamano wrote:\n\n> > I wonder how we are supposed to use this trailer in the Git project,\n> > in particular in combination with Signed-off-by.  Should all\n> > (co)authors sign off as well?  Or will Co-authored-by imply\n> > Signed-off-by?\n> \n> I think we have been happy with (1) a comment at the end of the log\n> message that says X worked together with Y and Z to produce this\n> patch, and (2) the trailer block that has S-o-b: from X, Y and Z,\n> without any need for Co-authored-by: trailer so far, and I do not\n> see any reason to change it in this project.\n\nOne advantage to making a machine-readable version is that tools on the\nreading side can then count contributions, etc. For instance:\n\n  https://github.com/git/git/commit/69f272b922df153c86db520bf9b6018a9808c2a6\n\nshows all of the co-author avatars, and you can click through to their\npages.\n\n> If other projects wants to use such a footer, that's their choice,\n> but I am fairly negative to the idea to open the gate to unbounded\n> number of new options for new trailer lines.  We do not even have\n> such options like --acked=<acker>, --reported=<reporter>, for the\n> trailers that are actively used already (and to make sure nobody\n> misunderstands, I do not think it is a good idea to add such\n> individual options).\n\nYeah, I'd agree that we should start first with a generic trailer line.\nThere might be some advantage to building trailer-specific intelligence\non top of that (for instance, it would be nice for coauthor trailers to\nexpand names the way --author does). But that can come after, and might\nnot even be in the form of specific command-line options. E.g., if the\ncoauthor trailer could be marked in config as \"this is an ident\", then\nwe we would know to expand it. And the same could apply to acked,\nreported, etc.\n\n-Peff\n"},{"id":"383782","messageId":"87sgo1q92k.fsf@iotcl.com","threadId":"51993","inReplyTo":"20191009203105.GA7007@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] commit: add support to provide --coauthor","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2019-10-10T08:49:23Z","receivedAt":"2019-10-10T08:49:35Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Jeff King <peff@peff.net> writes: \n \n> Yeah, I'd agree that we should start first with a generic \n> trailer line.\n\nIIUC you are suggesting something like this?\n\n  git commit --trailer=\"Co-authored-by: <coauthor>\"\n\nI really want to consider this, but I do not understand how that \nimproves the user experience compared to adding that trailer \nmanually when typing the commit message in your $EDITOR?\n\n> There might be some advantage to building trailer-specific \n> intelligence on top of that (for instance, it would be nice for \n> coauthor trailers to expand names the way --author does). But \n> that can come after, and might not even be in the form of \n> specific command-line options. E.g., if the coauthor trailer \n> could be marked in config as \"this is an ident\", then we we \n> would know to expand it. And the same could apply to acked, \n> reported, etc.\n\nWouldn't making it a generic --trailer option make this more \ncomplex? I can image users might even want to use the --trailer \nargument to indicate which issue the commit closes:\n\n  git commit --trailer=\"Closes: $BUGNUMBER\"\n\nSo, how can we make the config understand it has to expand \nCo-authored-by and not Closes?\n\nMaybe, because I was looking at \nhttps://git.wiki.kernel.org/index.php/CommitMessageConventions#Trailers,\nit will probably be safe to expand a name when a '-by' suffix is \nused.\n\nWith this pattern in place there is a slight improvement compared \nto typing the trailer in your $EDITOR, because the user can pass \n--trailer=\"Anything-by: name\" and the trailer is expanded to \n`Anything-by: name <name@example.com>\".\n\nBut I'd like to note another thing, and let me circle back to \nSZEDER Gábor's reply:\n\n> I wonder how we are supposed to use this trailer in the Git \n> project, in particular in combination with Signed-off-by. \n> Should all (co)authors sign off as well?  Or will Co-authored-by \n> imply Signed-off-by?\n\nFor this purpose I think it's useful git understands what \n\"Co-authored-by\" means, so when you run: \n\n  git commit -s --coauthor=<coauthor>\n\nThe following trailer will be added:\n\n  Co-authored-by: <coauthor>\n  Signed-off-by: <author>\n  Signed-off-by: <coauthor>\n\nSo I'm still pro of adding a --co-author option, but I do \nunderstand the concerns to avoid adding an option for all the \npossible trailers found in the link above.\n\n-- \nToon\n"},{"id":"383791","messageId":"nycvar.QRO.7.76.6.1910101345120.46@tvgsbejvaqbjf.bet","threadId":"51993","inReplyTo":"20191009203105.GA7007@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] commit: add support to provide --coauthor","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-10T11:49:03Z","receivedAt":"2019-10-10T11:49:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 9 Oct 2019, Jeff King wrote:\n\n> On Wed, Oct 09, 2019 at 11:19:47AM +0900, Junio C Hamano wrote:\n>\n> > > I wonder how we are supposed to use this trailer in the Git project,\n> > > in particular in combination with Signed-off-by.  Should all\n> > > (co)authors sign off as well?  Or will Co-authored-by imply\n> > > Signed-off-by?\n> >\n> > I think we have been happy with (1) a comment at the end of the log\n> > message that says X worked together with Y and Z to produce this\n> > patch, and (2) the trailer block that has S-o-b: from X, Y and Z,\n> > without any need for Co-authored-by: trailer so far, and I do not\n> > see any reason to change it in this project.\n>\n> One advantage to making a machine-readable version is that tools on the\n> reading side can then count contributions, etc. For instance:\n>\n>   https://github.com/git/git/commit/69f272b922df153c86db520bf9b6018a9808c2a6\n>\n> shows all of the co-author avatars, and you can click through to their\n> pages.\n\nFWIW I really like this. It bugged me ever since that GitMerge talk\n(https://www.youtube.com/watch?v=usQgAy8YDVA) that we did not have any\nstandardized way to document co-authored commits.\n\n> > If other projects wants to use such a footer, that's their choice,\n> > but I am fairly negative to the idea to open the gate to unbounded\n> > number of new options for new trailer lines.  We do not even have\n> > such options like --acked=<acker>, --reported=<reporter>, for the\n> > trailers that are actively used already (and to make sure nobody\n> > misunderstands, I do not think it is a good idea to add such\n> > individual options).\n>\n> Yeah, I'd agree that we should start first with a generic trailer line.\n> There might be some advantage to building trailer-specific intelligence\n> on top of that (for instance, it would be nice for coauthor trailers to\n> expand names the way --author does). But that can come after, and might\n> not even be in the form of specific command-line options. E.g., if the\n> coauthor trailer could be marked in config as \"this is an ident\", then\n> we we would know to expand it. And the same could apply to acked,\n> reported, etc.\n\nYep, and we have to start somewhere. I think this patch is a good start.\n\nFWIW I would not even mind introducing the synonym `--co-author` for\n`--coauthor`. But that's just a very minor suggestion.\n\nCiao,\nDscho\n"},{"id":"383816","messageId":"20191010163755.GA12756@sigill.intra.peff.net","threadId":"51993","inReplyTo":"87sgo1q92k.fsf@iotcl.com","subject":"Re: [PATCH 1/1] commit: add support to provide --coauthor","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-10-10T16:37:56Z","receivedAt":"2019-10-10T16:37:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 10, 2019 at 10:49:23AM +0200, Toon Claes wrote:\n\n> > Yeah, I'd agree that we should start first with a generic trailer line.\n> \n> IIUC you are suggesting something like this?\n> \n>  git commit --trailer=\"Co-authored-by: <coauthor>\"\n> \n> I really want to consider this, but I do not understand how that improves\n> the user experience compared to adding that trailer manually when typing the\n> commit message in your $EDITOR?\n\nI agree that it's a lot worse to type than \"--coauthor\". And I don't\nreally have a problem with us ending up with \"--coauthor\". My reasoning\nin starting with a generic form was mostly:\n\n  - by having _any_ way to do this on the command-line, it makes it\n    possible to use in aliases, etc.\n\n  - having a generic form, even if we later add syntactic sugar on\n    top, lets people easily experiment with their own trailers\n\n> > There might be some advantage to building trailer-specific intelligence\n> > on top of that (for instance, it would be nice for coauthor trailers to\n> > expand names the way --author does). But that can come after, and might\n> > not even be in the form of specific command-line options. E.g., if the\n> > coauthor trailer could be marked in config as \"this is an ident\", then\n> > we we would know to expand it. And the same could apply to acked,\n> > reported, etc.\n> \n> Wouldn't making it a generic --trailer option make this more complex? I can\n> image users might even want to use the --trailer argument to indicate which\n> issue the commit closes:\n> \n>  git commit --trailer=\"Closes: $BUGNUMBER\"\n> \n> So, how can we make the config understand it has to expand Co-authored-by\n> and not Closes?\n\nWe already have config blocks for specific trailers to describe how they\nshould be parsed or added. I was thinking that you'd set an option like\n\"trailer.co-authored-by.ident\" to \"true\". And possibly that could be\nused in other places, too (e.g., interpret-trailers code could make sure\nit's syntactically valid, but I didn't really think through the\nimplications there).\n\nAnd of course we could bake in the defaults for particular trailers if\nwe wanted to (I think we already do for trailer.signoff.*).\n\n> > I wonder how we are supposed to use this trailer in the Git project, in\n> > particular in combination with Signed-off-by. Should all (co)authors\n> > sign off as well?  Or will Co-authored-by imply Signed-off-by?\n> \n> For this purpose I think it's useful git understands what \"Co-authored-by\"\n> means, so when you run:\n> \n>  git commit -s --coauthor=<coauthor>\n> \n> The following trailer will be added:\n> \n>  Co-authored-by: <coauthor>\n>  Signed-off-by: <author>\n>  Signed-off-by: <coauthor>\n> \n> So I'm still pro of adding a --co-author option, but I do understand the\n> concerns to avoid adding an option for all the possible trailers found in\n> the link above.\n\nYes, I agree that ordering and de-duplication rules are useful, too.\nSome of that can be expressed already in trailer.* config, but I don't\nknow if it would be capable enough to do everything you want (though\nagain, it would be really nice to _make_ it capable enough so that other\ntypes besides co-authored-by can make use of them).\n\nI don't have a hard belief that we have to do it that way (generic\nbefore specific), and I can believe that when you get down to the\ndetails that it might be hard to express some of this stuff in config\nrather than C code. But I think we should at least take a look at\nwhether it's possible, because the benefits of having a generic solution\nare nice.\n\n-Peff\n"},{"id":"383819","messageId":"20191010170032.GA78298@generichostname","threadId":"51993","inReplyTo":"nycvar.QRO.7.76.6.1910101345120.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/1] commit: add support to provide --coauthor","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-10-10T17:00:32Z","receivedAt":"2019-10-10T17:00:37Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"On Thu, Oct 10, 2019 at 01:49:03PM +0200, Johannes Schindelin wrote:\n> Hi,\n> \n> On Wed, 9 Oct 2019, Jeff King wrote:\n> \n> > On Wed, Oct 09, 2019 at 11:19:47AM +0900, Junio C Hamano wrote:\n> >\n> > > > I wonder how we are supposed to use this trailer in the Git project,\n> > > > in particular in combination with Signed-off-by.  Should all\n> > > > (co)authors sign off as well?  Or will Co-authored-by imply\n> > > > Signed-off-by?\n> > >\n> > > I think we have been happy with (1) a comment at the end of the log\n> > > message that says X worked together with Y and Z to produce this\n> > > patch, and (2) the trailer block that has S-o-b: from X, Y and Z,\n> > > without any need for Co-authored-by: trailer so far, and I do not\n> > > see any reason to change it in this project.\n> >\n> > One advantage to making a machine-readable version is that tools on the\n> > reading side can then count contributions, etc. For instance:\n> >\n> >   https://github.com/git/git/commit/69f272b922df153c86db520bf9b6018a9808c2a6\n> >\n> > shows all of the co-author avatars, and you can click through to their\n> > pages.\n\nYep, this is the reason why I raised the suggestion[1] in the\nfirst place. Since special support for this trailer is implemented in\nGitHub (and GitLab too, as I recently learned), I think this could be\nconsidered a defacto standard for co-authored commits.\n\n> \n> FWIW I really like this. It bugged me ever since that GitMerge talk\n> (https://www.youtube.com/watch?v=usQgAy8YDVA) that we did not have any\n> standardized way to document co-authored commits.\n\nYep, and this isn't the first time this has been brought up. I remember\nstumbling on this thread[2] a while back about someone asking for\nco-author functionality by default so it would be nice to have a\nstandard way of doing it.\n\nThanks,\n\nDenton\n\n[1]: https://github.com/gitgitgadget/git/issues/343\n[2]: https://public-inbox.org/git/CAOvwQ4i_HL7XGnxZrVu3oSnsbnTyxbg8Vh6vzi4c1isSrrexYQ@mail.gmail.com/\n\n> \n> > > If other projects wants to use such a footer, that's their choice,\n> > > but I am fairly negative to the idea to open the gate to unbounded\n> > > number of new options for new trailer lines.  We do not even have\n> > > such options like --acked=<acker>, --reported=<reporter>, for the\n> > > trailers that are actively used already (and to make sure nobody\n> > > misunderstands, I do not think it is a good idea to add such\n> > > individual options).\n> >\n> > Yeah, I'd agree that we should start first with a generic trailer line.\n> > There might be some advantage to building trailer-specific intelligence\n> > on top of that (for instance, it would be nice for coauthor trailers to\n> > expand names the way --author does). But that can come after, and might\n> > not even be in the form of specific command-line options. E.g., if the\n> > coauthor trailer could be marked in config as \"this is an ident\", then\n> > we we would know to expand it. And the same could apply to acked,\n> > reported, etc.\n> \n> Yep, and we have to start somewhere. I think this patch is a good start.\n> \n> FWIW I would not even mind introducing the synonym `--co-author` for\n> `--coauthor`. But that's just a very minor suggestion.\n> \n> Ciao,\n> Dscho\n"},{"id":"383847","messageId":"20191010230716.rylvyiyqu4gv4zcc@camp.crustytoothpaste.net","threadId":"51993","inReplyTo":"87sgo1q92k.fsf@iotcl.com","subject":"Re: [PATCH 1/1] commit: add support to provide --coauthor","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-10-10T23:07:17Z","receivedAt":"2019-10-10T23:07:24Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-10-10 at 08:49:23, Toon Claes wrote:\n> Jeff King <peff@peff.net> writes:\n> \n> > Yeah, I'd agree that we should start first with a generic trailer line.\n> \n> IIUC you are suggesting something like this?\n> \n>  git commit --trailer=\"Co-authored-by: <coauthor>\"\n> \n> I really want to consider this, but I do not understand how that improves\n> the user experience compared to adding that trailer manually when typing the\n> commit message in your $EDITOR?\n\nThe --trailer option to git interpret-trailers knows how to interpret\nconfiguration options and expand them.  For example, you could\nabbreviate \"Co-authored-by\" as \"cab\", and if you used that alias, you\ncould write \"git commit --trailer='cab=peff'\" and then have your command\nlook up \"peff\" and find the proper identification either from your repo\nor from your Git hosting solution of choice (or wherever).\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"383858","messageId":"xmqqwodbj52u.fsf@gitster-ct.c.googlers.com","threadId":"51993","inReplyTo":"20191010163755.GA12756@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] commit: add support to provide --coauthor","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-11T04:09:45Z","receivedAt":"2019-10-11T04:09:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Yes, I agree that ordering and de-duplication rules are useful, too.\n> Some of that can be expressed already in trailer.* config, but I don't\n> know if it would be capable enough to do everything you want (though\n> again, it would be really nice to _make_ it capable enough so that other\n> types besides co-authored-by can make use of them).\n\nYup, absolutely.\n\nAs a mature system, unlike the early days, randomly adding a\n\"desired\" feature without first considering if a generalized form\ncan be added is simply irresponsible to our users.  Response by\nBrian on the side thread also indicates that we have already spent\nenough effort to the generalized trailer support and building on\ntop, instead of randomly adding an ad-hoc \"feature\", may be the\nright and feasible approach.\n\n"}]}