| Message ID | pull.901.git.1615446968597.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [GSOC] commit: provides multiple common signatures | expand |
Hey! The idea seems very useful to me though I am not sure what others feel about this and whether Git is ready for such a thing or not. Keeping this concern aside, I will still review the patch due to my own interest in it as well. On 11/03 07:16, ZheNing Hu via GitGitGadget wrote: > From: ZheNing Hu <adlternative@gmail.com> > > Similar to "Helped-by", "Reported-by", "Reviewed-by", "Mentored-by" > these signatures are often seen in git commit messages. After I think it will be better to rephrase the line so that it is easier to understand. It took me a couple of reads to figure out what you meant. Something like: Similar to "Signed-off-by", trailers such as "Reported-by", "Helped-by" and "Mentored-by" are also seen in Git commits. > referring to the simple implementation of `commit --signoff` > and `send-email -cc=" commiter <email>"`, I am considering > whether to provide multiple signature parameters from the > command line. I think this might help maintainers and > developers directly uses the shell to provide these signatures > instead of multiple times repetitive writing those trailers > each time. The above para is more appropriate for a cover letter than a commit message. Your thought process for the patch you sent is equally valuable but this does not belong in the commit message. Commit messages are more about what you did and any nuances that follow. You get me? Use 'git format-patch --cover-letter <...>' to create a cover letter for your patch. > To achieve this goal, i refactored the `append_signoff` design and > provided `append_message` and `append_message_string_list` interfaces, > providing new ways to generate those various signatures. s/i/I Also, commit messages are generally written in the imperative tense when desciribing what you have done in the commit. > Users now can use `commit -H "helper <eamil>"` to generate "Helped-by" trailer, > `commit -R "reviewer <eamil>"` to generate "Reviewed-by" trailer, > `commit -r "reporter <eamil> "`to generate "Reported-by" trailer, > `commit -M "mentor <eamil>"` to generate "Mentored-by" trailer. Multiple typos. > Signed-off-by: ZheNing Hu <adlternative@gmail.com> So, an improved commit message could be: -----8<----- commit: support multiple trailers from the command line Historically, Git has supported the 'Signed-off-by' commit trailer using the '--signoff' and the '-s' option from the command line. Extend this list to include other commonly used trailers viz., "Helped-by", "Reviewed-by", "Reported-by" and "Mentored-by". Introduce options '-H', '-R', '-r' and '-M' corresponding to the aforementioned trailers. While at it, add tests in the script 't7502-commit-porcelain.sh' and add information regarding these options in the Documentation of 'git commit'. ----->8----- > [GSOC] commit: provides multiple signatures from command line > > I don’t know if my idea will satisfy everyone, I'm also thinking about > whether we can provide a more generalized version (I think this idea can > be extended: because users and developers have other signature methods > that they want, such as "Based-on-patch-by") I hope someone can give me > pointers (on the correctness of ideas or codes) It will be great if we let the user add customised options for the respective trailers but I think that trying to cater to such a large audience will only complicate the whole process and decrease the support avialable for the options/command. Apart from this, I think that the trailer "Reviewed-by" may not be that widely used and it would be better if we remove it from this patch. > Documentation/git-commit.txt | 24 +++++++- > builtin/commit.c | 63 +++++++++++++++++++++ > sequencer.c | 40 +++++++++---- > sequencer.h | 4 ++ > t/t7502-commit-porcelain.sh | 106 +++++++++++++++++++++++++++++++++++ > 5 files changed, 226 insertions(+), 11 deletions(-) > > diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt > index 17150fa7eabe..e1b528d70c1a 100644 > --- a/Documentation/git-commit.txt > +++ b/Documentation/git-commit.txt > @@ -14,7 +14,9 @@ SYNOPSIS > [--allow-empty-message] [--no-verify] [-e] [--author=<author>] > [--date=<date>] [--cleanup=<mode>] [--[no-]status] > [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]] > - [-S[<keyid>]] [--] [<pathspec>...] > + [-S[<keyid>]] [--] [<pathspec>...] [(-H|--helped-by)=<address>...] > + [(-R|--reviewed-by)=<address>...] [(-r|--reported-by)=<address>...] > + [(-M|--mentored)=<address>...] > > DESCRIPTION > ----------- > @@ -166,6 +168,26 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`. > > include::signoff-option.txt[] > > +-H=<address>...:: > +--helped-by=<address>...:: > + Add one or more `Helped-by` trailer by the committer at the end of the commit > + log message. > + > +-R=<address>...:: > +--reviewed-by=<address>...:: > + Add one or more `Reviewed-by` trailer by the committer at the end of the commit > + log message. > + > +-r=<address>...:: > +--reported-by=<address>...:: > + Add one or more `Reported-by` trailer by the committer at the end of the commit > + log message. > + > +-M=<address>...:: > +--mentored-by=<address>...:: > + Add one or more `Mentored-by` trailer by the committer at the end of the commit > + log message. Oh! I did not think you had added long forms of these options and was about to comment on that. Good that you added. Do talk about them as well in the commit message. > -n:: > --no-verify:: > This option bypasses the pre-commit and commit-msg hooks. > diff --git a/builtin/commit.c b/builtin/commit.c > index 739110c5a7f6..4b312af03181 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -113,6 +113,10 @@ static int config_commit_verbose = -1; /* unspecified */ > static int no_post_rewrite, allow_empty_message, pathspec_file_nul; > static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg; > static char *sign_commit, *pathspec_from_file; > +static struct string_list helped_by = STRING_LIST_INIT_NODUP; > +static struct string_list mentored_by = STRING_LIST_INIT_NODUP; > +static struct string_list reviewed_by = STRING_LIST_INIT_NODUP; > +static struct string_list reported_by = STRING_LIST_INIT_NODUP; Good that you kept the variables as 'string_list's instead of a usual 'char*'. > /* > * The default commit message cleanup mode will remove the lines > @@ -829,6 +833,20 @@ static int prepare_to_commit(const char *index_file, const char *prefix, > if (signoff) > append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0); > > + if(helped_by.items) > + append_message_string_list(&sb, "Helped-by: ", &helped_by, ignore_non_trailer(sb.buf, sb.len), 0); > + if(reviewed_by.items) > + append_message_string_list(&sb, "Reviewed-by: ", &reviewed_by, ignore_non_trailer(sb.buf, sb.len), 0); > + if(reported_by.items) > + append_message_string_list(&sb, "Reported-by: ", &reported_by, ignore_non_trailer(sb.buf, sb.len), 0); > + if(mentored_by.items) > + append_message_string_list(&sb, "Mentored-by: ", &mentored_by, ignore_non_trailer(sb.buf, sb.len), 0); I think this part will look prettier if you wrap around the text. For code segments, the wrap around limit is 80 chars. For commit messages its 72 chars. Wrap around means that as soon as you hit N number of characters, you proceed to the next(new) line. > + string_list_clear(&helped_by, 0); > + string_list_clear(&reviewed_by, 0); > + string_list_clear(&reported_by, 0); > + string_list_clear(&mentored_by, 0); > + > if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len) > die_errno(_("could not write commit template")); > > @@ -1490,6 +1508,42 @@ static int git_commit_config(const char *k, const char *v, void *cb) > return git_status_config(k, v, s); > } > > +static int help_callback(const struct option *opt, const char *arg, int unset) > +{ > + if (unset) > + string_list_clear(&helped_by, 0); > + else > + string_list_append(&helped_by, arg); > + return 0; > +} > + > +static int review_callback(const struct option *opt, const char *arg, int unset) > +{ > + if (unset) > + string_list_clear(&reviewed_by, 0); > + else > + string_list_append(&reviewed_by, arg); > + return 0; > +} > + > +static int report_callback(const struct option *opt, const char *arg, int unset) > +{ > + if (unset) > + string_list_clear(&reported_by, 0); > + else > + string_list_append(&reported_by, arg); > + return 0; > +} > + > +static int mentor_callback(const struct option *opt, const char *arg, int unset) > +{ > + if (unset) > + string_list_clear(&mentored_by, 0); > + else > + string_list_append(&mentored_by, arg); > + return 0; > +} > int cmd_commit(int argc, const char **argv, const char *prefix) > { > static struct wt_status s; > @@ -1507,6 +1561,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix) > OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")), > 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_CALLBACK('H', "helped-by", NULL, N_("email"), N_("add a Helped-by trailer"), help_callback), > + OPT_CALLBACK('r', "reported-by", NULL, N_("email"), N_("add a Reported-by trailer"), report_callback), > + OPT_CALLBACK('R', "reviewed-by", NULL, N_("email"), N_("add a Reviewed-by trailer"), review_callback), > + OPT_CALLBACK('M', "mentored-by", NULL, N_("email"), N_("add a Mentored-by trailer"), mentor_callback), It would be lovely if you could line wrap these above additions too! > OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")), > OPT_FILENAME('t', "template", &template_file, N_("use specified template file")), > OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")), > @@ -1561,6 +1619,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix) > struct commit_extra_header *extra = NULL; > struct strbuf err = STRBUF_INIT; > > + helped_by.strdup_strings = 1; > + reviewed_by.strdup_strings = 1; > + reported_by.strdup_strings = 1; > + mentored_by.strdup_strings = 1; > + > if (argc == 2 && !strcmp(argv[1], "-h")) > usage_with_options(builtin_commit_usage, builtin_commit_options); > > diff --git a/sequencer.c b/sequencer.c > index d2332d3e1787..528daf9df252 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -4668,16 +4668,12 @@ int sequencer_pick_revisions(struct repository *r, > return res; > } > > -void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) > +void append_message(struct strbuf *msgbuf, struct strbuf *sob, > + size_t ignore_footer, unsigned flag) Its nice to see that you generalised the pre-exisiting function instead of creating a new one(s) for the new trailers. Good. It will be better to name 'struct strbuf *sob' to something more generic, maybe 'struct strbuf *trailer' since 'sob' stands for 'Signed-off-by' and since the function is becoming more generic, its contents should too. > { > 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'); > - > if (!ignore_footer) > strbuf_complete_line(msgbuf); > > @@ -4685,11 +4681,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); Again, rename the variables. > if (!has_footer) { > const char *append_newlines = NULL; > @@ -4723,8 +4719,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) > +{ > + struct strbuf sob = STRBUF_INIT; > + strbuf_addstr(&sob, sign_off_header); > + strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT)); > + strbuf_addch(&sob, '\n'); > + append_message(msgbuf, &sob, ignore_footer, flag); > + strbuf_release(&sob); > +} I think it will be nice if you could use a flag to denote whether the entity to be appended is a 'sign-off' or not. This way when say the variable 'int signoff' is 1, then use the above part in the 'append_message()' function otherwise go with the flow that exists. > +void append_message_string_list(struct strbuf *msgbuf, const char *header, > + struct string_list *sobs, size_t ignore_footer, unsigned flag) { > + int i; > + struct strbuf sob = STRBUF_INIT; > + > + for ( i = 0; i < sobs->nr; i++) > + { > + strbuf_addstr(&sob, header); > + strbuf_addstr(&sob, sobs->items[i].string); > + strbuf_addch(&sob, '\n'); > + append_message(msgbuf, &sob, ignore_footer, flag); > + strbuf_reset(&sob); > + } > strbuf_release(&sob); > } Similarly, here, if 'signoff = 0' then the above part goes on. Or another alternative can be to create a 'append_message_helper()' and shift the current contents of 'append_message()' into that and use the above function to work as per the value of signoff. So a possible flow can be: void append_message_string_list(...params...) { if (signoff) { ..excecute the necessary segments and call the 'append_message_helper()' function... } else { ..similarly here.. } } This way we save ourselves some extra functions. > diff --git a/sequencer.h b/sequencer.h > index f8b2e4ab8527..b24e274f4c62 100644 > --- a/sequencer.h > +++ b/sequencer.h > @@ -174,6 +174,10 @@ int todo_list_rearrange_squash(struct todo_list *todo_list); > * and the new signoff will be spliced into the buffer before those bytes. > */ > void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag); > +void append_message(struct strbuf *msgbuf, struct strbuf *sob, > + size_t ignore_footer, unsigned flag); > +void append_message_string_list(struct strbuf *msgbuf, const char*header, > + struct string_list *sobs, size_t ignore_footer, unsigned flag); > > void append_conflicts_hint(struct index_state *istate, > struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode); Then, these would have to be corrected too. > diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh > index 6396897cc818..40823152a51c 100755 > --- a/t/t7502-commit-porcelain.sh > +++ b/t/t7502-commit-porcelain.sh > @@ -154,6 +154,112 @@ test_expect_success 'sign off' ' > > ' I feel that the tests deserve a commit of their own. A follow-up commit in this patch. > +test_expect_success 'helped-by' ' > + > + >file1 && > + git add file1 && > + git commit --helped-by="foo <bar@frotz>" \ > + --helped-by="foo2 <bar2@frotz>" -m "thank you" && > + git cat-file commit HEAD >commit.msg && > + sed -ne "s/Helped-by: //p" commit.msg >actual && > + cat >expected <<-\EOF && > + foo <bar@frotz> > + foo2 <bar2@frotz> > + EOF > + test_cmp expected actual > + Was this extra line feed intentional? It looks odd here and other test (scripts) don't have this. Though, this test script seems to have many tests which have an extra line feed. My suggestion would be to eliminate it. Maybe this script is old and hasn't been reviewed for a cleanup in a long time. > +' > + > +test_expect_success 'reported-by' ' > + > + >file2 && > + git add file2 && > + git commit --reported-by="foo <bar@frotz>" \ > + --reported-by="foo2 <bar2@frotz>" -m "thank you" && > + git cat-file commit HEAD >commit.msg && > + sed -ne "s/Reported-by: //p" commit.msg >actual && > + cat >expected <<-\EOF && > + foo <bar@frotz> > + foo2 <bar2@frotz> > + EOF > + test_cmp expected actual > + Similarly here and in the parts that follow. <...> This seems like a good patch to me but more experienced members will be able to comment better I think. Good job anyways. Regards, Shourya Shukla
"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: ZheNing Hu <adlternative@gmail.com> > > Similar to "Helped-by", "Reported-by", "Reviewed-by", "Mentored-by" > these signatures are often seen in git commit messages. After > referring to the simple implementation of `commit --signoff` > and `send-email -cc=" commiter <email>"`, I am considering > whether to provide multiple signature parameters from the > command line. > + ... > + >file6 && > + git add file6 && > + git commit -H "foo <bar@frotz>" \ > + -R "foo2 <bar2@frotz>" \ > + -M "foo3 <bar3@frotz>" \ > + -r "foo4 <bar4@frotz>" -s -m "thank you" && Firm NAK. Especially, not in this form that squats on short-and-sweet single letter option names only to support the convention of this single project (namely, Git). cf. https://lore.kernel.org/git/20200824061156.1929850-1-espeer@gmail.com/ Thanks.
Shourya Shukla <periperidip@gmail.com> 于2021年3月11日周四 下午11:05写道： > > Hey! > Hi! > The idea seems very useful to me though I am not sure what others feel > about this and whether Git is ready for such a thing or not. Keeping > this concern aside, I will still review the patch due to my own interest > in it as well. > haha, thank you! I'm glad that my whims can be recognized by you and help git itself. > On 11/03 07:16, ZheNing Hu via GitGitGadget wrote: > > From: ZheNing Hu <adlternative@gmail.com> > > > > Similar to "Helped-by", "Reported-by", "Reviewed-by", "Mentored-by" > > these signatures are often seen in git commit messages. After > > I think it will be better to rephrase the line so that it is easier to > understand. It took me a couple of reads to figure out what you meant. > > Something like: > > Similar to "Signed-off-by", trailers such as "Reported-by", "Helped-by" > and "Mentored-by" are also seen in Git commits. > It's exactly what you said. My lack of English sometimes limits my expression. > > referring to the simple implementation of `commit --signoff` > > and `send-email -cc=" commiter <email>"`, I am considering > > whether to provide multiple signature parameters from the > > command line. I think this might help maintainers and > > developers directly uses the shell to provide these signatures > > instead of multiple times repetitive writing those trailers > > each time. > > The above para is more appropriate for a cover letter than a commit > message. Your thought process for the patch you sent is equally valuable > but this does not belong in the commit message. Commit messages are more > about what you did and any nuances that follow. You get me? > > Use 'git format-patch --cover-letter <...>' to create a cover letter for > your patch. > I understand it now, because I use GGG, I will put this content into the GGG conversation. > > To achieve this goal, i refactored the `append_signoff` design and > > provided `append_message` and `append_message_string_list` interfaces, > > providing new ways to generate those various signatures. > > s/i/I > > Also, commit messages are generally written in the imperative tense when > desciribing what you have done in the commit. > You're right. > > Users now can use `commit -H "helper <eamil>"` to generate "Helped-by" trailer, > > `commit -R "reviewer <eamil>"` to generate "Reviewed-by" trailer, > > `commit -r "reporter <eamil> "`to generate "Reported-by" trailer, > > `commit -M "mentor <eamil>"` to generate "Mentored-by" trailer. > > Multiple typos. > > > Signed-off-by: ZheNing Hu <adlternative@gmail.com> > > So, an improved commit message could be: > > -----8<----- > commit: support multiple trailers from the command line > > Historically, Git has supported the 'Signed-off-by' commit trailer > using the '--signoff' and the '-s' option from the command line. Extend > this list to include other commonly used trailers viz., "Helped-by", > "Reviewed-by", "Reported-by" and "Mentored-by". Introduce options '-H', > '-R', '-r' and '-M' corresponding to the aforementioned trailers. > > While at it, add tests in the script 't7502-commit-porcelain.sh' and add > information regarding these options in the Documentation of 'git > commit'. I think this paragraph may not be needed. Junio said in reply to one of my previous patches that we may not need to add instructions about test files and documents, because `git log -p --stat` can clearly see that what we have done in testing and documentation. > ----->8----- > > > [GSOC] commit: provides multiple signatures from command line > > > > I don’t know if my idea will satisfy everyone, I'm also thinking about > > whether we can provide a more generalized version (I think this idea can > > be extended: because users and developers have other signature methods > > that they want, such as "Based-on-patch-by") I hope someone can give me > > pointers (on the correctness of ideas or codes) > > It will be great if we let the user add customised options for the > respective trailers but I think that trying to cater to such a large > audience will only complicate the whole process and decrease the support > avialable for the options/command. Apart from this, I think that the > trailer "Reviewed-by" may not be that widely used and it would be better > if we remove it from this patch. Okay, I will delete it. > > > Documentation/git-commit.txt | 24 +++++++- > > builtin/commit.c | 63 +++++++++++++++++++++ > > sequencer.c | 40 +++++++++---- > > sequencer.h | 4 ++ > > t/t7502-commit-porcelain.sh | 106 +++++++++++++++++++++++++++++++++++ > > 5 files changed, 226 insertions(+), 11 deletions(-) > > > > diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt > > index 17150fa7eabe..e1b528d70c1a 100644 > > --- a/Documentation/git-commit.txt > > +++ b/Documentation/git-commit.txt > > @@ -14,7 +14,9 @@ SYNOPSIS > > [--allow-empty-message] [--no-verify] [-e] [--author=<author>] > > [--date=<date>] [--cleanup=<mode>] [--[no-]status] > > [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]] > > - [-S[<keyid>]] [--] [<pathspec>...] > > + [-S[<keyid>]] [--] [<pathspec>...] [(-H|--helped-by)=<address>...] > > + [(-R|--reviewed-by)=<address>...] [(-r|--reported-by)=<address>...] > > + [(-M|--mentored)=<address>...] > > > > DESCRIPTION > > ----------- > > @@ -166,6 +168,26 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`. > > > > include::signoff-option.txt[] > > > > +-H=<address>...:: > > +--helped-by=<address>...:: > > + Add one or more `Helped-by` trailer by the committer at the end of the commit > > + log message. > > + > > +-R=<address>...:: > > +--reviewed-by=<address>...:: > > + Add one or more `Reviewed-by` trailer by the committer at the end of the commit > > + log message. > > + > > +-r=<address>...:: > > +--reported-by=<address>...:: > > + Add one or more `Reported-by` trailer by the committer at the end of the commit > > + log message. > > + > > +-M=<address>...:: > > +--mentored-by=<address>...:: > > + Add one or more `Mentored-by` trailer by the committer at the end of the commit > > + log message. > > Oh! I did not think you had added long forms of these options and was > about to comment on that. Good that you added. Do talk about them as > well in the commit message. > Yes, I forgot to mention these long formats in the commit message. > > -n:: > > --no-verify:: > > This option bypasses the pre-commit and commit-msg hooks. > > diff --git a/builtin/commit.c b/builtin/commit.c > > index 739110c5a7f6..4b312af03181 100644 > > --- a/builtin/commit.c > > +++ b/builtin/commit.c > > @@ -113,6 +113,10 @@ static int config_commit_verbose = -1; /* unspecified */ > > static int no_post_rewrite, allow_empty_message, pathspec_file_nul; > > static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg; > > static char *sign_commit, *pathspec_from_file; > > +static struct string_list helped_by = STRING_LIST_INIT_NODUP; > > +static struct string_list mentored_by = STRING_LIST_INIT_NODUP; > > +static struct string_list reviewed_by = STRING_LIST_INIT_NODUP; > > +static struct string_list reported_by = STRING_LIST_INIT_NODUP; > > Good that you kept the variables as 'string_list's instead of a usual > 'char*'. > > > /* > > * The default commit message cleanup mode will remove the lines > > @@ -829,6 +833,20 @@ static int prepare_to_commit(const char *index_file, const char *prefix, > > if (signoff) > > append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0); > > > > + if(helped_by.items) > > + append_message_string_list(&sb, "Helped-by: ", &helped_by, ignore_non_trailer(sb.buf, sb.len), 0); > > + if(reviewed_by.items) > > + append_message_string_list(&sb, "Reviewed-by: ", &reviewed_by, ignore_non_trailer(sb.buf, sb.len), 0); > > + if(reported_by.items) > > + append_message_string_list(&sb, "Reported-by: ", &reported_by, ignore_non_trailer(sb.buf, sb.len), 0); > > + if(mentored_by.items) > > + append_message_string_list(&sb, "Mentored-by: ", &mentored_by, ignore_non_trailer(sb.buf, sb.len), 0); > > I think this part will look prettier if you wrap around the text. For > code segments, the wrap around limit is 80 chars. For commit messages > its 72 chars. Wrap around means that as soon as you hit N number of > characters, you proceed to the next(new) line. > Fine. > > + string_list_clear(&helped_by, 0); > > + string_list_clear(&reviewed_by, 0); > > + string_list_clear(&reported_by, 0); > > + string_list_clear(&mentored_by, 0); > > + > > if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len) > > die_errno(_("could not write commit template")); > > > > @@ -1490,6 +1508,42 @@ static int git_commit_config(const char *k, const char *v, void *cb) > > return git_status_config(k, v, s); > > } > > > > +static int help_callback(const struct option *opt, const char *arg, int unset) > > +{ > > + if (unset) > > + string_list_clear(&helped_by, 0); > > + else > > + string_list_append(&helped_by, arg); > > + return 0; > > +} > > + > > +static int review_callback(const struct option *opt, const char *arg, int unset) > > +{ > > + if (unset) > > + string_list_clear(&reviewed_by, 0); > > + else > > + string_list_append(&reviewed_by, arg); > > + return 0; > > +} > > + > > +static int report_callback(const struct option *opt, const char *arg, int unset) > > +{ > > + if (unset) > > + string_list_clear(&reported_by, 0); > > + else > > + string_list_append(&reported_by, arg); > > + return 0; > > +} > > + > > +static int mentor_callback(const struct option *opt, const char *arg, int unset) > > +{ > > + if (unset) > > + string_list_clear(&mentored_by, 0); > > + else > > + string_list_append(&mentored_by, arg); > > + return 0; > > +} > > int cmd_commit(int argc, const char **argv, const char *prefix) > > { > > static struct wt_status s; > > @@ -1507,6 +1561,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix) > > OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")), > > 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_CALLBACK('H', "helped-by", NULL, N_("email"), N_("add a Helped-by trailer"), help_callback), > > + OPT_CALLBACK('r', "reported-by", NULL, N_("email"), N_("add a Reported-by trailer"), report_callback), > > + OPT_CALLBACK('R', "reviewed-by", NULL, N_("email"), N_("add a Reviewed-by trailer"), review_callback), > > + OPT_CALLBACK('M', "mentored-by", NULL, N_("email"), N_("add a Mentored-by trailer"), mentor_callback), > > It would be lovely if you could line wrap these above additions too! > > > OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")), > > OPT_FILENAME('t', "template", &template_file, N_("use specified template file")), > > OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")), > > @@ -1561,6 +1619,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix) > > struct commit_extra_header *extra = NULL; > > struct strbuf err = STRBUF_INIT; > > > > + helped_by.strdup_strings = 1; > > + reviewed_by.strdup_strings = 1; > > + reported_by.strdup_strings = 1; > > + mentored_by.strdup_strings = 1; > > + > > if (argc == 2 && !strcmp(argv[1], "-h")) > > usage_with_options(builtin_commit_usage, builtin_commit_options); > > > > diff --git a/sequencer.c b/sequencer.c > > index d2332d3e1787..528daf9df252 100644 > > --- a/sequencer.c > > +++ b/sequencer.c > > @@ -4668,16 +4668,12 @@ int sequencer_pick_revisions(struct repository *r, > > return res; > > } > > > > -void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) > > +void append_message(struct strbuf *msgbuf, struct strbuf *sob, > > + size_t ignore_footer, unsigned flag) > > Its nice to see that you generalised the pre-exisiting function instead > of creating a new one(s) for the new trailers. Good. > > It will be better to name 'struct strbuf *sob' to something more > generic, maybe 'struct strbuf *trailer' since 'sob' stands for > 'Signed-off-by' and since the function is becoming more generic, its > contents should too. > When I wrote it, I didn’t realize that sob refers to the abbreviation of 'Signed-off-by', I will change it. > > { > > 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'); > > - > > if (!ignore_footer) > > strbuf_complete_line(msgbuf); > > > > @@ -4685,11 +4681,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); > > Again, rename the variables. > > > if (!has_footer) { > > const char *append_newlines = NULL; > > @@ -4723,8 +4719,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) > > +{ > > + struct strbuf sob = STRBUF_INIT; > > + strbuf_addstr(&sob, sign_off_header); > > + strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT)); > > + strbuf_addch(&sob, '\n'); > > + append_message(msgbuf, &sob, ignore_footer, flag); > > + strbuf_release(&sob); > > +} > > I think it will be nice if you could use a flag to denote whether the > entity to be appended is a 'sign-off' or not. This way when say the > variable 'int signoff' is 1, then use the above part in the > 'append_message()' function otherwise go with the flow that exists. > > > +void append_message_string_list(struct strbuf *msgbuf, const char *header, > > + struct string_list *sobs, size_t ignore_footer, unsigned flag) { > > + int i; > > + struct strbuf sob = STRBUF_INIT; > > + > > + for ( i = 0; i < sobs->nr; i++) > > + { > > + strbuf_addstr(&sob, header); > > + strbuf_addstr(&sob, sobs->items[i].string); > > + strbuf_addch(&sob, '\n'); > > + append_message(msgbuf, &sob, ignore_footer, flag); > > + strbuf_reset(&sob); > > + } > > strbuf_release(&sob); > > } > > Similarly, here, if 'signoff = 0' then the above part goes on. Or > another alternative can be to create a 'append_message_helper()' and > shift the current contents of 'append_message()' into that and use the > above function to work as per the value of signoff. > > So a possible flow can be: > > void append_message_string_list(...params...) { > > if (signoff) { > ..excecute the necessary segments and call the > 'append_message_helper()' function... > } else { > ..similarly here.. > } > } > > This way we save ourselves some extra functions. May some thing like this: if (signoff) append_message_string_list(&sb, "Signed-off-by: ", NULL, ignore_non_trailer(sb.buf, sb.len), 0); And then we could judge in `append_message_string_list` if the arguement `string_list *trailers` set to NULL. If so, we do something similar to `append_signoff`, otherwise, we carry out other situations. So that we can delete the `append_signoff` and user can only need to call `append_message_string_list` in any where, I don't know if it is better. > > > diff --git a/sequencer.h b/sequencer.h > > index f8b2e4ab8527..b24e274f4c62 100644 > > --- a/sequencer.h > > +++ b/sequencer.h > > @@ -174,6 +174,10 @@ int todo_list_rearrange_squash(struct todo_list *todo_list); > > * and the new signoff will be spliced into the buffer before those bytes. > > */ > > void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag); > > +void append_message(struct strbuf *msgbuf, struct strbuf *sob, > > + size_t ignore_footer, unsigned flag); > > +void append_message_string_list(struct strbuf *msgbuf, const char*header, > > + struct string_list *sobs, size_t ignore_footer, unsigned flag); > > > > void append_conflicts_hint(struct index_state *istate, > > struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode); > > Then, these would have to be corrected too. > > > diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh > > index 6396897cc818..40823152a51c 100755 > > --- a/t/t7502-commit-porcelain.sh > > +++ b/t/t7502-commit-porcelain.sh > > @@ -154,6 +154,112 @@ test_expect_success 'sign off' ' > > > > ' > > I feel that the tests deserve a commit of their own. A follow-up commit > in this patch. > > > +test_expect_success 'helped-by' ' > > + > > + >file1 && > > + git add file1 && > > + git commit --helped-by="foo <bar@frotz>" \ > > + --helped-by="foo2 <bar2@frotz>" -m "thank you" && > > + git cat-file commit HEAD >commit.msg && > > + sed -ne "s/Helped-by: //p" commit.msg >actual && > > + cat >expected <<-\EOF && > > + foo <bar@frotz> > > + foo2 <bar2@frotz> > > + EOF > > + test_cmp expected actual > > + > > Was this extra line feed intentional? It looks odd here and other test > (scripts) don't have this. Though, this test script seems to have many > tests which have an extra line feed. My suggestion would be to eliminate > it. Maybe this script is old and hasn't been reviewed for a cleanup in > a long time. > I might want to keep the same format with the last "sign off" test. Now it seems that this is not necessary. > > +' > > + > > +test_expect_success 'reported-by' ' > > + > > + >file2 && > > + git add file2 && > > + git commit --reported-by="foo <bar@frotz>" \ > > + --reported-by="foo2 <bar2@frotz>" -m "thank you" && > > + git cat-file commit HEAD >commit.msg && > > + sed -ne "s/Reported-by: //p" commit.msg >actual && > > + cat >expected <<-\EOF && > > + foo <bar@frotz> > > + foo2 <bar2@frotz> > > + EOF > > + test_cmp expected actual > > + > > Similarly here and in the parts that follow. > > <...> > > This seems like a good patch to me but more experienced members will be > able to comment better I think. Good job anyways. > Your comments and encouragement are of great help to me :) > Regards, > Shourya Shukla > Regards, ZheNing Hu
Junio C Hamano <gitster@pobox.com> 于2021年3月12日周五 上午1:29写道： > Firm NAK. > > Especially, not in this form that squats on short-and-sweet single > letter option names only to support the convention of this single > project (namely, Git). > > cf. https://lore.kernel.org/git/20200824061156.1929850-1-espeer@gmail.com/ > > Thanks. Well, Junio, you point is It's also worth thinking about, This "trailer =<trailer>:<argtrailer>" is probably the more general version of the tailer I want to provide to commit, Then I may need to know how Christian Couder `trailer.c` work first,or can you give a little directionality guidance? Thanks.
Junio C Hamano <gitster@pobox.com> 于2021年3月12日周五 上午1:29写道： > Firm NAK. > > Especially, not in this form that squats on short-and-sweet single > letter option names only to support the convention of this single > project (namely, Git). > > cf. https://lore.kernel.org/git/20200824061156.1929850-1-espeer@gmail.com/ > > Thanks. Now I understand that We have a git command now: `git interpret-trailers --in-place --where=end \ ./a.txt --trailer "Signed-off-by:adl <adlternative@gmail.com> " ` can add the trailers to specified file, so now maybe I only need to provide `--trailer` for commit, right?
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 17150fa7eabe..e1b528d70c1a 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -14,7 +14,9 @@ SYNOPSIS [--allow-empty-message] [--no-verify] [-e] [--author=<author>] [--date=<date>] [--cleanup=<mode>] [--[no-]status] [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]] - [-S[<keyid>]] [--] [<pathspec>...] + [-S[<keyid>]] [--] [<pathspec>...] [(-H|--helped-by)=<address>...] + [(-R|--reviewed-by)=<address>...] [(-r|--reported-by)=<address>...] + [(-M|--mentored)=<address>...] DESCRIPTION ----------- @@ -166,6 +168,26 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`. include::signoff-option.txt[] +-H=<address>...:: +--helped-by=<address>...:: + Add one or more `Helped-by` trailer by the committer at the end of the commit + log message. + +-R=<address>...:: +--reviewed-by=<address>...:: + Add one or more `Reviewed-by` trailer by the committer at the end of the commit + log message. + +-r=<address>...:: +--reported-by=<address>...:: + Add one or more `Reported-by` trailer by the committer at the end of the commit + log message. + +-M=<address>...:: +--mentored-by=<address>...:: + Add one or more `Mentored-by` trailer by the committer at the end of the commit + log message. + -n:: --no-verify:: This option bypasses the pre-commit and commit-msg hooks. diff --git a/builtin/commit.c b/builtin/commit.c index 739110c5a7f6..4b312af03181 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -113,6 +113,10 @@ static int config_commit_verbose = -1; /* unspecified */ static int no_post_rewrite, allow_empty_message, pathspec_file_nul; static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg; static char *sign_commit, *pathspec_from_file; +static struct string_list helped_by = STRING_LIST_INIT_NODUP; +static struct string_list mentored_by = STRING_LIST_INIT_NODUP; +static struct string_list reviewed_by = STRING_LIST_INIT_NODUP; +static struct string_list reported_by = STRING_LIST_INIT_NODUP; /* * The default commit message cleanup mode will remove the lines @@ -829,6 +833,20 @@ static int prepare_to_commit(const char *index_file, const char *prefix, if (signoff) append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0); + if(helped_by.items) + append_message_string_list(&sb, "Helped-by: ", &helped_by, ignore_non_trailer(sb.buf, sb.len), 0); + if(reviewed_by.items) + append_message_string_list(&sb, "Reviewed-by: ", &reviewed_by, ignore_non_trailer(sb.buf, sb.len), 0); + if(reported_by.items) + append_message_string_list(&sb, "Reported-by: ", &reported_by, ignore_non_trailer(sb.buf, sb.len), 0); + if(mentored_by.items) + append_message_string_list(&sb, "Mentored-by: ", &mentored_by, ignore_non_trailer(sb.buf, sb.len), 0); + + string_list_clear(&helped_by, 0); + string_list_clear(&reviewed_by, 0); + string_list_clear(&reported_by, 0); + string_list_clear(&mentored_by, 0); + if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len) die_errno(_("could not write commit template")); @@ -1490,6 +1508,42 @@ static int git_commit_config(const char *k, const char *v, void *cb) return git_status_config(k, v, s); } +static int help_callback(const struct option *opt, const char *arg, int unset) +{ + if (unset) + string_list_clear(&helped_by, 0); + else + string_list_append(&helped_by, arg); + return 0; +} + +static int review_callback(const struct option *opt, const char *arg, int unset) +{ + if (unset) + string_list_clear(&reviewed_by, 0); + else + string_list_append(&reviewed_by, arg); + return 0; +} + +static int report_callback(const struct option *opt, const char *arg, int unset) +{ + if (unset) + string_list_clear(&reported_by, 0); + else + string_list_append(&reported_by, arg); + return 0; +} + +static int mentor_callback(const struct option *opt, const char *arg, int unset) +{ + if (unset) + string_list_clear(&mentored_by, 0); + else + string_list_append(&mentored_by, arg); + return 0; +} + int cmd_commit(int argc, const char **argv, const char *prefix) { static struct wt_status s; @@ -1507,6 +1561,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix) OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")), 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_CALLBACK('H', "helped-by", NULL, N_("email"), N_("add a Helped-by trailer"), help_callback), + OPT_CALLBACK('r', "reported-by", NULL, N_("email"), N_("add a Reported-by trailer"), report_callback), + OPT_CALLBACK('R', "reviewed-by", NULL, N_("email"), N_("add a Reviewed-by trailer"), review_callback), + OPT_CALLBACK('M', "mentored-by", NULL, N_("email"), N_("add a Mentored-by trailer"), mentor_callback), OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")), OPT_FILENAME('t', "template", &template_file, N_("use specified template file")), OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")), @@ -1561,6 +1619,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix) struct commit_extra_header *extra = NULL; struct strbuf err = STRBUF_INIT; + helped_by.strdup_strings = 1; + reviewed_by.strdup_strings = 1; + reported_by.strdup_strings = 1; + mentored_by.strdup_strings = 1; + if (argc == 2 && !strcmp(argv[1], "-h")) usage_with_options(builtin_commit_usage, builtin_commit_options); diff --git a/sequencer.c b/sequencer.c index d2332d3e1787..528daf9df252 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4668,16 +4668,12 @@ int sequencer_pick_revisions(struct repository *r, return res; } -void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) +void append_message(struct strbuf *msgbuf, struct strbuf *sob, + size_t ignore_footer, unsigned flag) { 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'); - if (!ignore_footer) strbuf_complete_line(msgbuf); @@ -4685,11 +4681,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; @@ -4723,8 +4719,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) +{ + struct strbuf sob = STRBUF_INIT; + strbuf_addstr(&sob, sign_off_header); + strbuf_addstr(&sob, fmt_name(WANT_COMMITTER_IDENT)); + strbuf_addch(&sob, '\n'); + append_message(msgbuf, &sob, ignore_footer, flag); + strbuf_release(&sob); +} +void append_message_string_list(struct strbuf *msgbuf, const char *header, + struct string_list *sobs, size_t ignore_footer, unsigned flag) { + int i; + struct strbuf sob = STRBUF_INIT; + + for ( i = 0; i < sobs->nr; i++) + { + strbuf_addstr(&sob, header); + strbuf_addstr(&sob, sobs->items[i].string); + strbuf_addch(&sob, '\n'); + append_message(msgbuf, &sob, ignore_footer, flag); + strbuf_reset(&sob); + } strbuf_release(&sob); } diff --git a/sequencer.h b/sequencer.h index f8b2e4ab8527..b24e274f4c62 100644 --- a/sequencer.h +++ b/sequencer.h @@ -174,6 +174,10 @@ int todo_list_rearrange_squash(struct todo_list *todo_list); * and the new signoff will be spliced into the buffer before those bytes. */ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag); +void append_message(struct strbuf *msgbuf, struct strbuf *sob, + size_t ignore_footer, unsigned flag); +void append_message_string_list(struct strbuf *msgbuf, const char*header, + struct string_list *sobs, size_t ignore_footer, unsigned flag); void append_conflicts_hint(struct index_state *istate, struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode); diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh index 6396897cc818..40823152a51c 100755 --- a/t/t7502-commit-porcelain.sh +++ b/t/t7502-commit-porcelain.sh @@ -154,6 +154,112 @@ test_expect_success 'sign off' ' ' +test_expect_success 'helped-by' ' + + >file1 && + git add file1 && + git commit --helped-by="foo <bar@frotz>" \ + --helped-by="foo2 <bar2@frotz>" -m "thank you" && + git cat-file commit HEAD >commit.msg && + sed -ne "s/Helped-by: //p" commit.msg >actual && + cat >expected <<-\EOF && + foo <bar@frotz> + foo2 <bar2@frotz> + EOF + test_cmp expected actual + +' + +test_expect_success 'reported-by' ' + + >file2 && + git add file2 && + git commit --reported-by="foo <bar@frotz>" \ + --reported-by="foo2 <bar2@frotz>" -m "thank you" && + git cat-file commit HEAD >commit.msg && + sed -ne "s/Reported-by: //p" commit.msg >actual && + cat >expected <<-\EOF && + foo <bar@frotz> + foo2 <bar2@frotz> + EOF + test_cmp expected actual + +' + +test_expect_success 'reviewed-by' ' + + >file3 && + git add file3 && + git commit --reviewed-by="foo <bar@frotz>" \ + --reviewed-by="foo2 <bar2@frotz>" -m "thank you" && + git cat-file commit HEAD >commit.msg && + sed -ne "s/Reviewed-by: //p" commit.msg >actual && + cat >expected <<-\EOF && + foo <bar@frotz> + foo2 <bar2@frotz> + EOF + test_cmp expected actual + +' + +test_expect_success 'mentored-by' ' + + >file4 && + git add file4 && + git commit --mentored-by="foo <bar@frotz>" \ + --mentored-by="foo2 <bar2@frotz>" -m "thank you" && + git cat-file commit HEAD >commit.msg && + sed -ne "s/Mentored-by: //p" commit.msg >actual && + cat >expected <<-\EOF && + foo <bar@frotz> + foo2 <bar2@frotz> + EOF + test_cmp expected actual + +' + +test_expect_success 'multiple signatures' ' + + >file5 && + git add file5 && + git commit --helped-by="foo <bar@frotz>" \ + --reviewed-by="foo2 <bar2@frotz>" \ + --mentored-by="foo3 <bar3@frotz>" \ + --reported-by="foo4 <bar4@frotz>" -s -m "thank you" && + git cat-file commit HEAD >commit.msg && + sed -e "1,7d" commit.msg >actual && + cat >expected <<-\EOF && + Signed-off-by: C O Mitter <committer@example.com> + Helped-by: foo <bar@frotz> + Reviewed-by: foo2 <bar2@frotz> + Reported-by: foo4 <bar4@frotz> + Mentored-by: foo3 <bar3@frotz> + EOF + test_cmp expected actual + +' + +test_expect_success 'multiple signatures (use abbreviations)' ' + + >file6 && + git add file6 && + git commit -H "foo <bar@frotz>" \ + -R "foo2 <bar2@frotz>" \ + -M "foo3 <bar3@frotz>" \ + -r "foo4 <bar4@frotz>" -s -m "thank you" && + git cat-file commit HEAD >commit.msg && + sed -e "1,7d" commit.msg >actual && + cat >expected <<-\EOF && + Signed-off-by: C O Mitter <committer@example.com> + Helped-by: foo <bar@frotz> + Reviewed-by: foo2 <bar2@frotz> + Reported-by: foo4 <bar4@frotz> + Mentored-by: foo3 <bar3@frotz> + EOF + test_cmp expected actual + +' + test_expect_success 'multiple -m' ' >negative &&