| Message ID | 20191008074935.10972-1-toon@iotcl.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] commit: add support to provide --coauthor | expand |
Welcome to the Git community, Toon! Wow, I never realised that people actually read my braindump of issues on GGG. Thanks for taking this on. First some housekeeping, when formatting your patches, it's a good idea to use `git format-patch --thread` so that your patches are grouped together by thread. On Tue, Oct 08, 2019 at 09:49:35AM +0200, Toon Claes wrote: > Add support to provide the Co-author when committing. For each > co-author provided with --coauthor=<coauthor>, a line is added at the I know you're taking the name from my GGG issue but perhaps --co-author would look better? > bottom of the commit message, like this: > > Co-authored-by: <coauthor> > > It's a common practice use when pairing up with other people and both > authors want to in the commit message. > > Co-authored-by: Zeger-Jan van de Weg <git@zjvandeweg.nl> Nice ;) > Signed-off-by: Toon Claes <toon@iotcl.com> > --- > Documentation/git-commit.txt | 5 ++++ > builtin/commit.c | 7 ++++++ > sequencer.c | 44 ++++++++++++++++++++++++++---------- > sequencer.h | 2 ++ > t/t7502-commit-porcelain.sh | 11 +++++++++ > 5 files changed, 57 insertions(+), 12 deletions(-) > > diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt > index afa7b75a23..c059944e38 100644 > --- a/Documentation/git-commit.txt > +++ b/Documentation/git-commit.txt > @@ -140,6 +140,11 @@ OPTIONS > commit by that author (i.e. rev-list --all -i --author=<author>); > the commit author is then copied from the first such commit found. > > +--coauthor=<coauthor>:: > + Add a Co-authored-by line with the specified author. Specify the This line is indented with spaces but it should be changed to a single tab. > + author using the standard `Co Artur <co-artur@example.com>` > + format. > + > --date=<date>:: > Override the author date used in the commit. > > diff --git a/builtin/commit.c b/builtin/commit.c > index ae7aaf6dc6..feb423ed6f 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -110,6 +110,7 @@ static int config_commit_verbose = -1; /* unspecified */ > static int no_post_rewrite, allow_empty_message; > static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg; > static char *sign_commit; > +static struct string_list coauthors = STRING_LIST_INIT_NODUP; > > /* > * The default commit message cleanup mode will remove the lines > @@ -672,6 +673,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, > int clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE); > int old_display_comment_prefix; > int merge_contains_scissors = 0; > + int i; > > /* This checks and barfs if author is badly specified */ > determine_author_info(author_ident); > @@ -803,6 +805,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix, > if (clean_message_contents) > strbuf_stripspace(&sb, 0); > > + for (i = 0; i < coauthors.nr; i++) { > + append_coauthor(&sb, coauthors.items[i].string); > + } > + > if (signoff) > append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0); > > @@ -1504,6 +1510,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) > OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")), > OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")), > OPT_BOOL('s', "signoff", &signoff, N_("add Signed-off-by:")), > + OPT_STRING_LIST(0, "coauthor", &coauthors, N_("co-author"), N_("add Co-authored-by:")), > OPT_FILENAME('t', "template", &template_file, N_("use specified template file")), > OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")), > OPT_CLEANUP(&cleanup_arg), > diff --git a/sequencer.c b/sequencer.c > index d648aaf416..8958a22470 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -36,6 +36,7 @@ > #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" > > static const char sign_off_header[] = "Signed-off-by: "; > +static const char coauthor_header[] = "Co-authored-by: "; > static const char cherry_picked_prefix[] = "(cherry picked from commit "; > > GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG") > @@ -4385,15 +4386,9 @@ int sequencer_pick_revisions(struct repository *r, > return res; > } > > -void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) > +static void append_footer(struct strbuf *msgbuf, struct strbuf* sob, size_t ignore_footer, size_t no_dup_sob) The * for pointers should go with the name, not the type. Also, `sob` is a misleading name since it means "Signed-off-by". In this case, we're using it as a general footer (since it can include Co-author lines too) so perhaps rename this to `footer`? Finally, as a nit, can we mark sob as const? > { > - unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP; > - struct strbuf sob = STRBUF_INIT; > - int has_footer; > - > - strbuf_addstr(&sob, sign_off_header); > - strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT)); > - strbuf_addch(&sob, '\n'); > + size_t has_footer; Why was this changed into a size_t? has_conforming_footer() below returns int so we should leave it as is. > > if (!ignore_footer) > strbuf_complete_line(msgbuf); > @@ -4402,11 +4397,11 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) > * If the whole message buffer is equal to the sob, pretend that we > * found a conforming footer with a matching sob > */ > - if (msgbuf->len - ignore_footer == sob.len && > - !strncmp(msgbuf->buf, sob.buf, sob.len)) > + if (msgbuf->len - ignore_footer == sob->len && > + !strncmp(msgbuf->buf, sob->buf, sob->len)) > has_footer = 3; > else > - has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer); > + has_footer = has_conforming_footer(msgbuf, sob, ignore_footer); Since you're touching this area, could you please a prepatory patch before this one that changes the function signature: - static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, size_t ignore_footer) + static int has_conforming_footer(struct strbuf *sb, const struct strbuf *footer, size_t ignore_footer) That way, you'll be able to mark `sob` const in append_footer() and also, `sob` can be renamed to `footer` which should be less misleading. > > if (!has_footer) { > const char *append_newlines = NULL; > @@ -4440,7 +4435,32 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) > > if (has_footer != 3 && (!no_dup_sob || has_footer != 2)) > strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, > - sob.buf, sob.len); > + sob->buf, sob->len); > +} > + > +void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) > +{ > + unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP; > + struct strbuf sob = STRBUF_INIT; > + > + strbuf_addstr(&sob, sign_off_header); > + strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT)); > + strbuf_addch(&sob, '\n'); > + > + append_footer(msgbuf, &sob, ignore_footer, no_dup_sob); > + > + strbuf_release(&sob); > +} > + > +void append_coauthor(struct strbuf *msgbuf, const char *coauthor) > +{ > + struct strbuf sob = STRBUF_INIT; Same, this `sob` should definitely be written as `footer` or maybe `coauthor`. > + > + strbuf_addstr(&sob, coauthor_header); > + strbuf_addstr(&sob, coauthor); > + strbuf_addch(&sob, '\n'); > + > + append_footer(msgbuf, &sob, 0, 1); > > strbuf_release(&sob); > } > diff --git a/sequencer.h b/sequencer.h > index 574260f621..e36489fce7 100644 > --- a/sequencer.h > +++ b/sequencer.h > @@ -170,6 +170,8 @@ int todo_list_rearrange_squash(struct todo_list *todo_list); > */ > void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag); > > +void append_coauthor(struct strbuf *msgbuf, const char* co_author); Asterisk should also be attached to the name. > + > void append_conflicts_hint(struct index_state *istate, > struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode); > enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg, > diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh > index 14c92e4c25..5ed6735cf4 100755 > --- a/t/t7502-commit-porcelain.sh > +++ b/t/t7502-commit-porcelain.sh > @@ -138,6 +138,17 @@ test_expect_success 'partial removal' ' > > ' > > +test_expect_success 'co-author' ' > + > + >coauthor && > + git add coauthor && > + git commit -m "thank you" --co-author="Author <author@example.com>" && > + git cat-file commit HEAD >commit.msg && > + sed -ne "s/Co-authored-by: //p" commit.msg >actual && > + echo "Author <author@example.com>" >expected && > + test_cmp expected actual > +' > + This test looks good to me. Thanks again for taking this on, Denton > test_expect_success 'sign off' ' > > >positive && > -- > 2.22.0.rc3
Hi Toon & Zeger-Jan On 08/10/2019 08:49, Toon Claes wrote: > Add support to provide the Co-author when committing. For each > co-author provided with --coauthor=<coauthor>, a line is added at the > bottom of the commit message, like this: > > Co-authored-by: <coauthor> > > It's a common practice use when pairing up with other people and both > authors want to in the commit message. Thanks for the patch, it's looking good. I can see this being useful to some people - I like the way the patch itself is co-authored. > > Co-authored-by: Zeger-Jan van de Weg <git@zjvandeweg.nl> > Signed-off-by: Toon Claes <toon@iotcl.com> > --- > Documentation/git-commit.txt | 5 ++++ > builtin/commit.c | 7 ++++++ > sequencer.c | 44 ++++++++++++++++++++++++++---------- > sequencer.h | 2 ++ > t/t7502-commit-porcelain.sh | 11 +++++++++ > 5 files changed, 57 insertions(+), 12 deletions(-) > > diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt > index afa7b75a23..c059944e38 100644 > --- a/Documentation/git-commit.txt > +++ b/Documentation/git-commit.txt > @@ -140,6 +140,11 @@ OPTIONS > commit by that author (i.e. rev-list --all -i --author=<author>); > the commit author is then copied from the first such commit found. > > +--coauthor=<coauthor>:: > + Add a Co-authored-by line with the specified author. Specify the > + author using the standard `Co Artur <co-artur@example.com>` > + format. > + > --date=<date>:: > Override the author date used in the commit. > > diff --git a/builtin/commit.c b/builtin/commit.c > index ae7aaf6dc6..feb423ed6f 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -110,6 +110,7 @@ static int config_commit_verbose = -1; /* unspecified */ > static int no_post_rewrite, allow_empty_message; > static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg; > static char *sign_commit; > +static struct string_list coauthors = STRING_LIST_INIT_NODUP; > > /* > * The default commit message cleanup mode will remove the lines > @@ -672,6 +673,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, > int clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE); > int old_display_comment_prefix; > int merge_contains_scissors = 0; > + int i; > > /* This checks and barfs if author is badly specified */ > determine_author_info(author_ident); > @@ -803,6 +805,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix, > if (clean_message_contents) > strbuf_stripspace(&sb, 0); > > + for (i = 0; i < coauthors.nr; i++) { > + append_coauthor(&sb, coauthors.items[i].string); If you look at the existing code that's calling append_signoff() it does append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0) The purpose of ignore_non_trailer is to ignore comment and conflicts messages at the end of the commit message (there's more detail in a comment above it's definition in builtin/commit.c). I think we need to pass this offset when adding co-authors as well. One question - what is the desired de-duplication behavior of Co-authored-by:? What happens if there is already a matching Co-authored-by: footer? (It is also possible for the trailers code to only ignore an existing footer if it's the last one.) What happens if the same Co-authored-by: is duplicated on the command line? It would be nice to have this defined and tests to check it's enforced. Another useful addition would be to check that the footer value looks sane but that could come later - I don't think we do that for any other footers at the moment (though I haven't checked to see if that's really true) > + } > + > if (signoff) > append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0); > > @@ -1504,6 +1510,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) > OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")), > OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")), > OPT_BOOL('s', "signoff", &signoff, N_("add Signed-off-by:")), > + OPT_STRING_LIST(0, "coauthor", &coauthors, N_("co-author"), N_("add Co-authored-by:")), > OPT_FILENAME('t', "template", &template_file, N_("use specified template file")), > OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")), > OPT_CLEANUP(&cleanup_arg), > diff --git a/sequencer.c b/sequencer.c > index d648aaf416..8958a22470 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -36,6 +36,7 @@ > #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" > > static const char sign_off_header[] = "Signed-off-by: "; > +static const char coauthor_header[] = "Co-authored-by: "; > static const char cherry_picked_prefix[] = "(cherry picked from commit "; > > GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG") > @@ -4385,15 +4386,9 @@ int sequencer_pick_revisions(struct repository *r, > return res; > } > > -void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) > +static void append_footer(struct strbuf *msgbuf, struct strbuf* sob, size_t ignore_footer, size_t no_dup_sob) > { > - unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP; > - struct strbuf sob = STRBUF_INIT; > - int has_footer; > - > - strbuf_addstr(&sob, sign_off_header); > - strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT)); > - strbuf_addch(&sob, '\n'); > + size_t has_footer; > > if (!ignore_footer) > strbuf_complete_line(msgbuf); > @@ -4402,11 +4397,11 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) > * If the whole message buffer is equal to the sob, pretend that we > * found a conforming footer with a matching sob > */ > - if (msgbuf->len - ignore_footer == sob.len && > - !strncmp(msgbuf->buf, sob.buf, sob.len)) > + if (msgbuf->len - ignore_footer == sob->len && > + !strncmp(msgbuf->buf, sob->buf, sob->len)) > has_footer = 3; > else > - has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer); > + has_footer = has_conforming_footer(msgbuf, sob, ignore_footer); > > if (!has_footer) { > const char *append_newlines = NULL; > @@ -4440,7 +4435,32 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) > > if (has_footer != 3 && (!no_dup_sob || has_footer != 2)) > strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, > - sob.buf, sob.len); > + sob->buf, sob->len); > +} > + > +void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) > +{ > + unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP; > + struct strbuf sob = STRBUF_INIT; > + > + strbuf_addstr(&sob, sign_off_header); > + strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT)); > + strbuf_addch(&sob, '\n'); > + > + append_footer(msgbuf, &sob, ignore_footer, no_dup_sob); > + > + strbuf_release(&sob); > +} > + > +void append_coauthor(struct strbuf *msgbuf, const char *coauthor) > +{ > + struct strbuf sob = STRBUF_INIT; > + > + strbuf_addstr(&sob, coauthor_header); > + strbuf_addstr(&sob, coauthor); > + strbuf_addch(&sob, '\n'); > + > + append_footer(msgbuf, &sob, 0, 1); As we have a constant for APPEND_SIGNOFF_DEDUP can we use it here please rather than '1' which does not covey the same meaning to the author. Also as I said above I think you want to pass in a real value for ignore_footer not zero > > strbuf_release(&sob); > } > diff --git a/sequencer.h b/sequencer.h > index 574260f621..e36489fce7 100644 > --- a/sequencer.h > +++ b/sequencer.h > @@ -170,6 +170,8 @@ int todo_list_rearrange_squash(struct todo_list *todo_list); > */ > void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag); > > +void append_coauthor(struct strbuf *msgbuf, const char* co_author); > + > void append_conflicts_hint(struct index_state *istate, > struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode); > enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg, > diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh > index 14c92e4c25..5ed6735cf4 100755 > --- a/t/t7502-commit-porcelain.sh > +++ b/t/t7502-commit-porcelain.sh > @@ -138,6 +138,17 @@ test_expect_success 'partial removal' ' > > ' > > +test_expect_success 'co-author' ' > + > + >coauthor && > + git add coauthor && > + git commit -m "thank you" --co-author="Author <author@example.com>" && > + git cat-file commit HEAD >commit.msg && > + sed -ne "s/Co-authored-by: //p" commit.msg >actual && > + echo "Author <author@example.com>" >expected && > + test_cmp expected actual > +' This is fine as far as it goes but it would be nice to test the de-duplication behavior once that's defined Best Wishes Phillip > test_expect_success 'sign off' ' > > >positive && > -- > 2.22.0.rc3 >
On 08/10/2019 11:11, Phillip Wood wrote: > Hi Toon & Zeger-Jan > > On 08/10/2019 08:49, Toon Claes wrote: >> Add support to provide the Co-author when committing. For each >> co-author provided with --coauthor=<coauthor>, a line is added at the >> bottom of the commit message, like this: >> >> Co-authored-by: <coauthor> >> >> It's a common practice use when pairing up with other people and both >> authors want to in the commit message. > > Thanks for the patch, it's looking good. I can see this being useful to > some people - I like the way the patch itself is co-authored. > [...] >> @@ -803,6 +805,10 @@ static int prepare_to_commit(const char >> *index_file, const char *prefix, >> if (clean_message_contents) >> strbuf_stripspace(&sb, 0); >> >> + for (i = 0; i < coauthors.nr; i++) { >> + append_coauthor(&sb, coauthors.items[i].string); > > If you look at the existing code that's calling append_signoff() it does > append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0) > The purpose of ignore_non_trailer is to ignore comment and conflicts > messages at the end of the commit message (there's more detail in a > comment above it's definition in builtin/commit.c). I think we need to > pass this offset when adding co-authors as well. > > One question - what is the desired de-duplication behavior of > Co-authored-by:? What happens if there is already a matching > Co-authored-by: footer? (It is also possible for the trailers code to > only ignore an existing footer if it's the last one.) What happens if > the same Co-authored-by: is duplicated on the command line? It would be > nice to have this defined and tests to check it's enforced. I should give a bit more detail here. git-interpret-trailers gives more control over handling of duplicates that is configurable via 'git config' than 'commit --signoff' does. The reason for this is that 'commit --signoff' predates the interpret-trailers stuff. As we're adding a new footer command we should decide if we want it to act like --signoff or give the user the ability to configure the de-duplication behavior by using the interpret-trailers code path instead. (I think format-patch --signoff respects the interpret-trailers config variables but 'am --signoff' and 'cherry-pick --signoff' do not. > > Another useful addition would be to check that the footer value looks > sane but that could come later Looking at the way commit handles --author (grep for force_author in builtin/commit.c) it should be simple to add these checks - just call split_ident() and check it returns zero. --author also checks if the string contains '<' and if it doesn't it uses the string given on the command line to lookup a matching author in the commit log - that could be a nice feature to use here too (see the code that calls find_author_by_nickname()). Best Wishes Phillip - I don't think we do that for any other > footers at the moment (though I haven't checked to see if that's really > true) > >> + } >> + >> if (signoff) >> append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0); >> >> @@ -1504,6 +1510,7 @@ int cmd_commit(int argc, const char **argv, >> const char *prefix) >> OPT_STRING(0, "squash", &squash_message, N_("commit"), >> N_("use autosquash formatted message to squash specified commit")), >> OPT_BOOL(0, "reset-author", &renew_authorship, N_("the >> commit is authored by me now (used with -C/-c/--amend)")), >> OPT_BOOL('s', "signoff", &signoff, N_("add Signed-off-by:")), >> + OPT_STRING_LIST(0, "coauthor", &coauthors, N_("co-author"), >> N_("add Co-authored-by:")), >> OPT_FILENAME('t', "template", &template_file, N_("use >> specified template file")), >> OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")), >> OPT_CLEANUP(&cleanup_arg), >> diff --git a/sequencer.c b/sequencer.c >> index d648aaf416..8958a22470 100644 >> --- a/sequencer.c >> +++ b/sequencer.c >> @@ -36,6 +36,7 @@ >> #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" >> >> static const char sign_off_header[] = "Signed-off-by: "; >> +static const char coauthor_header[] = "Co-authored-by: "; >> static const char cherry_picked_prefix[] = "(cherry picked from >> commit "; >> >> GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG") >> @@ -4385,15 +4386,9 @@ int sequencer_pick_revisions(struct repository *r, >> return res; >> } >> >> -void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, >> unsigned flag) >> +static void append_footer(struct strbuf *msgbuf, struct strbuf* sob, >> size_t ignore_footer, size_t no_dup_sob) >> { >> - unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP; >> - struct strbuf sob = STRBUF_INIT; >> - int has_footer; >> - >> - strbuf_addstr(&sob, sign_off_header); >> - strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT)); >> - strbuf_addch(&sob, '\n'); >> + size_t has_footer; >> >> if (!ignore_footer) >> strbuf_complete_line(msgbuf); >> @@ -4402,11 +4397,11 @@ void append_signoff(struct strbuf *msgbuf, >> size_t ignore_footer, unsigned flag) >> * If the whole message buffer is equal to the sob, pretend that we >> * found a conforming footer with a matching sob >> */ >> - if (msgbuf->len - ignore_footer == sob.len && >> - !strncmp(msgbuf->buf, sob.buf, sob.len)) >> + if (msgbuf->len - ignore_footer == sob->len && >> + !strncmp(msgbuf->buf, sob->buf, sob->len)) >> has_footer = 3; >> else >> - has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer); >> + has_footer = has_conforming_footer(msgbuf, sob, ignore_footer); >> >> if (!has_footer) { >> const char *append_newlines = NULL; >> @@ -4440,7 +4435,32 @@ void append_signoff(struct strbuf *msgbuf, >> size_t ignore_footer, unsigned flag) >> >> if (has_footer != 3 && (!no_dup_sob || has_footer != 2)) >> strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, >> - sob.buf, sob.len); >> + sob->buf, sob->len); >> +} >> + >> +void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, >> unsigned flag) >> +{ >> + unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP; >> + struct strbuf sob = STRBUF_INIT; >> + >> + strbuf_addstr(&sob, sign_off_header); >> + strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT)); >> + strbuf_addch(&sob, '\n'); >> + >> + append_footer(msgbuf, &sob, ignore_footer, no_dup_sob); >> + >> + strbuf_release(&sob); >> +} >> + >> +void append_coauthor(struct strbuf *msgbuf, const char *coauthor) >> +{ >> + struct strbuf sob = STRBUF_INIT; >> + >> + strbuf_addstr(&sob, coauthor_header); >> + strbuf_addstr(&sob, coauthor); >> + strbuf_addch(&sob, '\n'); >> + >> + append_footer(msgbuf, &sob, 0, 1); > > As we have a constant for APPEND_SIGNOFF_DEDUP can we use it here please > rather than '1' which does not covey the same meaning to the author. > Also as I said above I think you want to pass in a real value for > ignore_footer not zero > >> >> strbuf_release(&sob); >> } >> diff --git a/sequencer.h b/sequencer.h >> index 574260f621..e36489fce7 100644 >> --- a/sequencer.h >> +++ b/sequencer.h >> @@ -170,6 +170,8 @@ int todo_list_rearrange_squash(struct todo_list >> *todo_list); >> */ >> void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, >> unsigned flag); >> >> +void append_coauthor(struct strbuf *msgbuf, const char* co_author); >> + >> void append_conflicts_hint(struct index_state *istate, >> struct strbuf *msgbuf, enum commit_msg_cleanup_mode >> cleanup_mode); >> enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg, >> diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh >> index 14c92e4c25..5ed6735cf4 100755 >> --- a/t/t7502-commit-porcelain.sh >> +++ b/t/t7502-commit-porcelain.sh >> @@ -138,6 +138,17 @@ test_expect_success 'partial removal' ' >> >> ' >> >> +test_expect_success 'co-author' ' >> + >> + >coauthor && >> + git add coauthor && >> + git commit -m "thank you" --co-author="Author >> <author@example.com>" && >> + git cat-file commit HEAD >commit.msg && >> + sed -ne "s/Co-authored-by: //p" commit.msg >actual && >> + echo "Author <author@example.com>" >expected && >> + test_cmp expected actual >> +' > > This is fine as far as it goes but it would be nice to test the > de-duplication behavior once that's defined > > Best Wishes > > Phillip > >> test_expect_success 'sign off' ' >> >> >positive && >> -- >> 2.22.0.rc3 >>
On Tue, Oct 08, 2019 at 09:49:35AM +0200, Toon Claes wrote: > Add support to provide the Co-author when committing. For each > co-author provided with --coauthor=<coauthor>, a line is added at the > bottom of the commit message, like this: > > Co-authored-by: <coauthor> > > It's a common practice use when pairing up with other people and both > authors want to in the commit message. I wonder how we are supposed to use this trailer in the Git project, in particular in combination with Signed-off-by. Should all (co)authors sign off as well? Or will Co-authored-by imply Signed-off-by?
SZEDER Gábor <szeder.dev@gmail.com> writes: > On Tue, Oct 08, 2019 at 09:49:35AM +0200, Toon Claes wrote: >> Add support to provide the Co-author when committing. For each >> co-author provided with --coauthor=<coauthor>, a line is added at the >> bottom of the commit message, like this: >> >> Co-authored-by: <coauthor> >> >> It's a common practice use when pairing up with other people and both >> authors want to in the commit message. > > I wonder how we are supposed to use this trailer in the Git project, > in particular in combination with Signed-off-by. Should all > (co)authors sign off as well? Or will Co-authored-by imply > Signed-off-by? I think we have been happy with (1) a comment at the end of the log message that says X worked together with Y and Z to produce this patch, and (2) the trailer block that has S-o-b: from X, Y and Z, without any need for Co-authored-by: trailer so far, and I do not see any reason to change it in this project. If other projects wants to use such a footer, that's their choice, but I am fairly negative to the idea to open the gate to unbounded number of new options for new trailer lines. We do not even have such options like --acked=<acker>, --reported=<reporter>, for the trailers that are actively used already (and to make sure nobody misunderstands, I do not think it is a good idea to add such individual options). Thanks.
On 2019-10-09 at 02:19:47, Junio C Hamano wrote: > I think we have been happy with (1) a comment at the end of the log > message that says X worked together with Y and Z to produce this > patch, and (2) the trailer block that has S-o-b: from X, Y and Z, > without any need for Co-authored-by: trailer so far, and I do not > see any reason to change it in this project. > > If other projects wants to use such a footer, that's their choice, > but I am fairly negative to the idea to open the gate to unbounded > number of new options for new trailer lines. We do not even have > such options like --acked=<acker>, --reported=<reporter>, for the > trailers that are actively used already (and to make sure nobody > misunderstands, I do not think it is a good idea to add such > individual options). If we don't want to add this option explicitly, which I understand, it may be useful to add an option to add arbitrary commit trailers to a commit message when creating it. Git uses a patch-based workflow, where adding trailers is relatively easy, but for folks using pull request-based workflows without signoffs, using Co-authored-by can be a great way to note when folks have worked together on a patch. An option to git commit such as --trailer would allow folks to add whatever trailers they wish, including this one, without us needing to bless particular options. I, for one, would be excited to see a series which did this.
"brian m. carlson" <sandals@crustytoothpaste.net> writes: > ... An option to git commit such as --trailer would > allow folks to add whatever trailers they wish, including this one, > without us needing to bless particular options. Yes, that was what I was hoping to become the "core" of the idea, with possibly syntax sugar to make it easier to use. Thanks for filling in what I left unsaid ;-)
On Wed, Oct 09, 2019 at 11:19:47AM +0900, Junio C Hamano wrote: > > I wonder how we are supposed to use this trailer in the Git project, > > in particular in combination with Signed-off-by. Should all > > (co)authors sign off as well? Or will Co-authored-by imply > > Signed-off-by? > > I think we have been happy with (1) a comment at the end of the log > message that says X worked together with Y and Z to produce this > patch, and (2) the trailer block that has S-o-b: from X, Y and Z, > without any need for Co-authored-by: trailer so far, and I do not > see any reason to change it in this project. One advantage to making a machine-readable version is that tools on the reading side can then count contributions, etc. For instance: https://github.com/git/git/commit/69f272b922df153c86db520bf9b6018a9808c2a6 shows all of the co-author avatars, and you can click through to their pages. > If other projects wants to use such a footer, that's their choice, > but I am fairly negative to the idea to open the gate to unbounded > number of new options for new trailer lines. We do not even have > such options like --acked=<acker>, --reported=<reporter>, for the > trailers that are actively used already (and to make sure nobody > misunderstands, I do not think it is a good idea to add such > individual options). Yeah, I'd agree that we should start first with a generic trailer line. There might be some advantage to building trailer-specific intelligence on top of that (for instance, it would be nice for coauthor trailers to expand names the way --author does). But that can come after, and might not even be in the form of specific command-line options. E.g., if the coauthor trailer could be marked in config as "this is an ident", then we we would know to expand it. And the same could apply to acked, reported, etc. -Peff
Jeff King <peff@peff.net> writes: > Yeah, I'd agree that we should start first with a generic > trailer line. IIUC you are suggesting something like this? git commit --trailer="Co-authored-by: <coauthor>" I really want to consider this, but I do not understand how that improves the user experience compared to adding that trailer manually when typing the commit message in your $EDITOR? > There might be some advantage to building trailer-specific > intelligence on top of that (for instance, it would be nice for > coauthor trailers to expand names the way --author does). But > that can come after, and might not even be in the form of > specific command-line options. E.g., if the coauthor trailer > could be marked in config as "this is an ident", then we we > would know to expand it. And the same could apply to acked, > reported, etc. Wouldn't making it a generic --trailer option make this more complex? I can image users might even want to use the --trailer argument to indicate which issue the commit closes: git commit --trailer="Closes: $BUGNUMBER" So, how can we make the config understand it has to expand Co-authored-by and not Closes? Maybe, because I was looking at https://git.wiki.kernel.org/index.php/CommitMessageConventions#Trailers, it will probably be safe to expand a name when a '-by' suffix is used. With this pattern in place there is a slight improvement compared to typing the trailer in your $EDITOR, because the user can pass --trailer="Anything-by: name" and the trailer is expanded to `Anything-by: name <name@example.com>". But I'd like to note another thing, and let me circle back to SZEDER Gábor's reply: > I wonder how we are supposed to use this trailer in the Git > project, in particular in combination with Signed-off-by. > Should all (co)authors sign off as well? Or will Co-authored-by > imply Signed-off-by? For this purpose I think it's useful git understands what "Co-authored-by" means, so when you run: git commit -s --coauthor=<coauthor> The following trailer will be added: Co-authored-by: <coauthor> Signed-off-by: <author> Signed-off-by: <coauthor> So I'm still pro of adding a --co-author option, but I do understand the concerns to avoid adding an option for all the possible trailers found in the link above.
Hi, On Wed, 9 Oct 2019, Jeff King wrote: > On Wed, Oct 09, 2019 at 11:19:47AM +0900, Junio C Hamano wrote: > > > > I wonder how we are supposed to use this trailer in the Git project, > > > in particular in combination with Signed-off-by. Should all > > > (co)authors sign off as well? Or will Co-authored-by imply > > > Signed-off-by? > > > > I think we have been happy with (1) a comment at the end of the log > > message that says X worked together with Y and Z to produce this > > patch, and (2) the trailer block that has S-o-b: from X, Y and Z, > > without any need for Co-authored-by: trailer so far, and I do not > > see any reason to change it in this project. > > One advantage to making a machine-readable version is that tools on the > reading side can then count contributions, etc. For instance: > > https://github.com/git/git/commit/69f272b922df153c86db520bf9b6018a9808c2a6 > > shows all of the co-author avatars, and you can click through to their > pages. FWIW I really like this. It bugged me ever since that GitMerge talk (https://www.youtube.com/watch?v=usQgAy8YDVA) that we did not have any standardized way to document co-authored commits. > > If other projects wants to use such a footer, that's their choice, > > but I am fairly negative to the idea to open the gate to unbounded > > number of new options for new trailer lines. We do not even have > > such options like --acked=<acker>, --reported=<reporter>, for the > > trailers that are actively used already (and to make sure nobody > > misunderstands, I do not think it is a good idea to add such > > individual options). > > Yeah, I'd agree that we should start first with a generic trailer line. > There might be some advantage to building trailer-specific intelligence > on top of that (for instance, it would be nice for coauthor trailers to > expand names the way --author does). But that can come after, and might > not even be in the form of specific command-line options. E.g., if the > coauthor trailer could be marked in config as "this is an ident", then > we we would know to expand it. And the same could apply to acked, > reported, etc. Yep, and we have to start somewhere. I think this patch is a good start. FWIW I would not even mind introducing the synonym `--co-author` for `--coauthor`. But that's just a very minor suggestion. Ciao, Dscho
On Thu, Oct 10, 2019 at 10:49:23AM +0200, Toon Claes wrote: > > Yeah, I'd agree that we should start first with a generic trailer line. > > IIUC you are suggesting something like this? > > git commit --trailer="Co-authored-by: <coauthor>" > > I really want to consider this, but I do not understand how that improves > the user experience compared to adding that trailer manually when typing the > commit message in your $EDITOR? I agree that it's a lot worse to type than "--coauthor". And I don't really have a problem with us ending up with "--coauthor". My reasoning in starting with a generic form was mostly: - by having _any_ way to do this on the command-line, it makes it possible to use in aliases, etc. - having a generic form, even if we later add syntactic sugar on top, lets people easily experiment with their own trailers > > There might be some advantage to building trailer-specific intelligence > > on top of that (for instance, it would be nice for coauthor trailers to > > expand names the way --author does). But that can come after, and might > > not even be in the form of specific command-line options. E.g., if the > > coauthor trailer could be marked in config as "this is an ident", then > > we we would know to expand it. And the same could apply to acked, > > reported, etc. > > Wouldn't making it a generic --trailer option make this more complex? I can > image users might even want to use the --trailer argument to indicate which > issue the commit closes: > > git commit --trailer="Closes: $BUGNUMBER" > > So, how can we make the config understand it has to expand Co-authored-by > and not Closes? We already have config blocks for specific trailers to describe how they should be parsed or added. I was thinking that you'd set an option like "trailer.co-authored-by.ident" to "true". And possibly that could be used in other places, too (e.g., interpret-trailers code could make sure it's syntactically valid, but I didn't really think through the implications there). And of course we could bake in the defaults for particular trailers if we wanted to (I think we already do for trailer.signoff.*). > > I wonder how we are supposed to use this trailer in the Git project, in > > particular in combination with Signed-off-by. Should all (co)authors > > sign off as well? Or will Co-authored-by imply Signed-off-by? > > For this purpose I think it's useful git understands what "Co-authored-by" > means, so when you run: > > git commit -s --coauthor=<coauthor> > > The following trailer will be added: > > Co-authored-by: <coauthor> > Signed-off-by: <author> > Signed-off-by: <coauthor> > > So I'm still pro of adding a --co-author option, but I do understand the > concerns to avoid adding an option for all the possible trailers found in > the link above. Yes, I agree that ordering and de-duplication rules are useful, too. Some of that can be expressed already in trailer.* config, but I don't know if it would be capable enough to do everything you want (though again, it would be really nice to _make_ it capable enough so that other types besides co-authored-by can make use of them). I don't have a hard belief that we have to do it that way (generic before specific), and I can believe that when you get down to the details that it might be hard to express some of this stuff in config rather than C code. But I think we should at least take a look at whether it's possible, because the benefits of having a generic solution are nice. -Peff
On Thu, Oct 10, 2019 at 01:49:03PM +0200, Johannes Schindelin wrote: > Hi, > > On Wed, 9 Oct 2019, Jeff King wrote: > > > On Wed, Oct 09, 2019 at 11:19:47AM +0900, Junio C Hamano wrote: > > > > > > I wonder how we are supposed to use this trailer in the Git project, > > > > in particular in combination with Signed-off-by. Should all > > > > (co)authors sign off as well? Or will Co-authored-by imply > > > > Signed-off-by? > > > > > > I think we have been happy with (1) a comment at the end of the log > > > message that says X worked together with Y and Z to produce this > > > patch, and (2) the trailer block that has S-o-b: from X, Y and Z, > > > without any need for Co-authored-by: trailer so far, and I do not > > > see any reason to change it in this project. > > > > One advantage to making a machine-readable version is that tools on the > > reading side can then count contributions, etc. For instance: > > > > https://github.com/git/git/commit/69f272b922df153c86db520bf9b6018a9808c2a6 > > > > shows all of the co-author avatars, and you can click through to their > > pages. Yep, this is the reason why I raised the suggestion[1] in the first place. Since special support for this trailer is implemented in GitHub (and GitLab too, as I recently learned), I think this could be considered a defacto standard for co-authored commits. > > FWIW I really like this. It bugged me ever since that GitMerge talk > (https://www.youtube.com/watch?v=usQgAy8YDVA) that we did not have any > standardized way to document co-authored commits. Yep, and this isn't the first time this has been brought up. I remember stumbling on this thread[2] a while back about someone asking for co-author functionality by default so it would be nice to have a standard way of doing it. Thanks, Denton [1]: https://github.com/gitgitgadget/git/issues/343 [2]: https://public-inbox.org/git/CAOvwQ4i_HL7XGnxZrVu3oSnsbnTyxbg8Vh6vzi4c1isSrrexYQ@mail.gmail.com/ > > > > If other projects wants to use such a footer, that's their choice, > > > but I am fairly negative to the idea to open the gate to unbounded > > > number of new options for new trailer lines. We do not even have > > > such options like --acked=<acker>, --reported=<reporter>, for the > > > trailers that are actively used already (and to make sure nobody > > > misunderstands, I do not think it is a good idea to add such > > > individual options). > > > > Yeah, I'd agree that we should start first with a generic trailer line. > > There might be some advantage to building trailer-specific intelligence > > on top of that (for instance, it would be nice for coauthor trailers to > > expand names the way --author does). But that can come after, and might > > not even be in the form of specific command-line options. E.g., if the > > coauthor trailer could be marked in config as "this is an ident", then > > we we would know to expand it. And the same could apply to acked, > > reported, etc. > > Yep, and we have to start somewhere. I think this patch is a good start. > > FWIW I would not even mind introducing the synonym `--co-author` for > `--coauthor`. But that's just a very minor suggestion. > > Ciao, > Dscho
On 2019-10-10 at 08:49:23, Toon Claes wrote: > Jeff King <peff@peff.net> writes: > > > Yeah, I'd agree that we should start first with a generic trailer line. > > IIUC you are suggesting something like this? > > git commit --trailer="Co-authored-by: <coauthor>" > > I really want to consider this, but I do not understand how that improves > the user experience compared to adding that trailer manually when typing the > commit message in your $EDITOR? The --trailer option to git interpret-trailers knows how to interpret configuration options and expand them. For example, you could abbreviate "Co-authored-by" as "cab", and if you used that alias, you could write "git commit --trailer='cab=peff'" and then have your command look up "peff" and find the proper identification either from your repo or from your Git hosting solution of choice (or wherever).
Jeff King <peff@peff.net> writes: > Yes, I agree that ordering and de-duplication rules are useful, too. > Some of that can be expressed already in trailer.* config, but I don't > know if it would be capable enough to do everything you want (though > again, it would be really nice to _make_ it capable enough so that other > types besides co-authored-by can make use of them). Yup, absolutely. As a mature system, unlike the early days, randomly adding a "desired" feature without first considering if a generalized form can be added is simply irresponsible to our users. Response by Brian on the side thread also indicates that we have already spent enough effort to the generalized trailer support and building on top, instead of randomly adding an ad-hoc "feature", may be the right and feasible approach.
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index afa7b75a23..c059944e38 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -140,6 +140,11 @@ OPTIONS commit by that author (i.e. rev-list --all -i --author=<author>); the commit author is then copied from the first such commit found. +--coauthor=<coauthor>:: + Add a Co-authored-by line with the specified author. Specify the + author using the standard `Co Artur <co-artur@example.com>` + format. + --date=<date>:: Override the author date used in the commit. diff --git a/builtin/commit.c b/builtin/commit.c index ae7aaf6dc6..feb423ed6f 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -110,6 +110,7 @@ static int config_commit_verbose = -1; /* unspecified */ static int no_post_rewrite, allow_empty_message; static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg; static char *sign_commit; +static struct string_list coauthors = STRING_LIST_INIT_NODUP; /* * The default commit message cleanup mode will remove the lines @@ -672,6 +673,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, int clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE); int old_display_comment_prefix; int merge_contains_scissors = 0; + int i; /* This checks and barfs if author is badly specified */ determine_author_info(author_ident); @@ -803,6 +805,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix, if (clean_message_contents) strbuf_stripspace(&sb, 0); + for (i = 0; i < coauthors.nr; i++) { + append_coauthor(&sb, coauthors.items[i].string); + } + if (signoff) append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0); @@ -1504,6 +1510,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")), OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")), OPT_BOOL('s', "signoff", &signoff, N_("add Signed-off-by:")), + OPT_STRING_LIST(0, "coauthor", &coauthors, N_("co-author"), N_("add Co-authored-by:")), OPT_FILENAME('t', "template", &template_file, N_("use specified template file")), OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")), OPT_CLEANUP(&cleanup_arg), diff --git a/sequencer.c b/sequencer.c index d648aaf416..8958a22470 100644 --- a/sequencer.c +++ b/sequencer.c @@ -36,6 +36,7 @@ #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" static const char sign_off_header[] = "Signed-off-by: "; +static const char coauthor_header[] = "Co-authored-by: "; static const char cherry_picked_prefix[] = "(cherry picked from commit "; GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG") @@ -4385,15 +4386,9 @@ int sequencer_pick_revisions(struct repository *r, return res; } -void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) +static void append_footer(struct strbuf *msgbuf, struct strbuf* sob, size_t ignore_footer, size_t no_dup_sob) { - unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP; - struct strbuf sob = STRBUF_INIT; - int has_footer; - - strbuf_addstr(&sob, sign_off_header); - strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT)); - strbuf_addch(&sob, '\n'); + size_t has_footer; if (!ignore_footer) strbuf_complete_line(msgbuf); @@ -4402,11 +4397,11 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) * If the whole message buffer is equal to the sob, pretend that we * found a conforming footer with a matching sob */ - if (msgbuf->len - ignore_footer == sob.len && - !strncmp(msgbuf->buf, sob.buf, sob.len)) + if (msgbuf->len - ignore_footer == sob->len && + !strncmp(msgbuf->buf, sob->buf, sob->len)) has_footer = 3; else - has_footer = has_conforming_footer(msgbuf, &sob, ignore_footer); + has_footer = has_conforming_footer(msgbuf, sob, ignore_footer); if (!has_footer) { const char *append_newlines = NULL; @@ -4440,7 +4435,32 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) if (has_footer != 3 && (!no_dup_sob || has_footer != 2)) strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, - sob.buf, sob.len); + sob->buf, sob->len); +} + +void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) +{ + unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP; + struct strbuf sob = STRBUF_INIT; + + strbuf_addstr(&sob, sign_off_header); + strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT)); + strbuf_addch(&sob, '\n'); + + append_footer(msgbuf, &sob, ignore_footer, no_dup_sob); + + strbuf_release(&sob); +} + +void append_coauthor(struct strbuf *msgbuf, const char *coauthor) +{ + struct strbuf sob = STRBUF_INIT; + + strbuf_addstr(&sob, coauthor_header); + strbuf_addstr(&sob, coauthor); + strbuf_addch(&sob, '\n'); + + append_footer(msgbuf, &sob, 0, 1); strbuf_release(&sob); } diff --git a/sequencer.h b/sequencer.h index 574260f621..e36489fce7 100644 --- a/sequencer.h +++ b/sequencer.h @@ -170,6 +170,8 @@ int todo_list_rearrange_squash(struct todo_list *todo_list); */ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag); +void append_coauthor(struct strbuf *msgbuf, const char* co_author); + void append_conflicts_hint(struct index_state *istate, struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode); enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg, diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh index 14c92e4c25..5ed6735cf4 100755 --- a/t/t7502-commit-porcelain.sh +++ b/t/t7502-commit-porcelain.sh @@ -138,6 +138,17 @@ test_expect_success 'partial removal' ' ' +test_expect_success 'co-author' ' + + >coauthor && + git add coauthor && + git commit -m "thank you" --co-author="Author <author@example.com>" && + git cat-file commit HEAD >commit.msg && + sed -ne "s/Co-authored-by: //p" commit.msg >actual && + echo "Author <author@example.com>" >expected && + test_cmp expected actual +' + test_expect_success 'sign off' ' >positive &&
Add support to provide the Co-author when committing. For each co-author provided with --coauthor=<coauthor>, a line is added at the bottom of the commit message, like this: Co-authored-by: <coauthor> It's a common practice use when pairing up with other people and both authors want to in the commit message. Co-authored-by: Zeger-Jan van de Weg <git@zjvandeweg.nl> Signed-off-by: Toon Claes <toon@iotcl.com> --- Documentation/git-commit.txt | 5 ++++ builtin/commit.c | 7 ++++++ sequencer.c | 44 ++++++++++++++++++++++++++---------- sequencer.h | 2 ++ t/t7502-commit-porcelain.sh | 11 +++++++++ 5 files changed, 57 insertions(+), 12 deletions(-) -- 2.22.0.rc3