Giuseppe Bilotta <giuseppe.bilo...@gmail.com> writes: > This makes it easy to sign off a whole patchset before submission. > > To make things work, we also fix a design issue in git-am that made it > ignore the signoff option during rebase (specifically, signoff was > handled in parse_mail(), but not in parse_mail_rebasing()).
I doubt that the above implementation detail in the code is "a design issue"; it is a logical consequence of a design whose "rebase" never passes "--signoff" down to underlying "am", so it is understandable that whoever wants to pass "--signoff" thru during the rebase needs to update the implementation, but I do not think it is fair to call that "an issue". > Documentation/git-rebase.txt | 5 +++++ > builtin/am.c | 6 +++--- > git-rebase.sh | 3 ++- > 3 files changed, 10 insertions(+), 4 deletions(-) We need new tests for "git rebase --signoff" that makes sure this works as expected and only when it should. > diff --git a/builtin/am.c b/builtin/am.c > index f7a7a971fb..d072027b5a 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -1321,9 +1321,6 @@ static int parse_mail(struct am_state *state, const > char *mail) > strbuf_addbuf(&msg, &mi.log_message); > strbuf_stripspace(&msg, 0); > > - if (state->signoff) > - am_signoff(&msg); > - > assert(!state->author_name); > state->author_name = strbuf_detach(&author_name, NULL); > > @@ -1848,6 +1845,9 @@ static void am_run(struct am_state *state, int resume) > if (skip) > goto next; /* mail should be skipped */ > > + if (state->signoff) > + am_append_signoff(state); > + > write_author_script(state); > write_commit_msg(state); > } This removes the last direct caller to am_signoff(). It may be worth considering to remove the function and move its body to its only internal caller am_append_signoff().
