Re: [PATCH v2] add --signoff flag to `git-merge` command.
Ævar Arnfjörð Bjarmasonwrites: > On Tue, Jul 04 2017, Łukasz Gryglicki jotted: > >> add --signoff flag to `git-merge` command. > > We'd usually say this as: > > merge: add a --signoff flag > > Or something like that. I thought I gave a fairly complete example that can be imitated, but apparently it didn't go through X-<. >> Some projects require every commit to be signed off. >> Our workflow is to create feature branches and require every commit to >> be signed off. When feature is finally approved we need to merge it into >> master. Merge itself is usually trivial and is done by >> `git merge origin/master`. Unfortunatelly this command have no --signoff >> flag, so we need to either add signoff line manually or use >> `git commit --amend -s` after the merge. First solution is not ideal >> because not all developers are familiar with exact sign-off syntax. >> The second solution works, but is obviously tedious. >> This patch adds --signoff support to git-merge command. It works just >> like --signoff in `git-commit` command. > > It would be nice to split this into a at least a couple of paragraphs, > and more closely follow the format suggested by > Documentation/SubmittingPatches. Good suggestion. >> More details here: >> https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-dy9h3kt4feymgv__p2gzznr0wua...@mail.gmail.com/T/#u > > These more details include my outstanding question in > 87fueferd4@gmail.com which hasn't been answered yet. I have an opinion on that topic, but I'd prefer to hear from others first before I speak out. >> diff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh >> new file mode 100755 >> index 0..f542f136f5dda >> --- /dev/null >> +++ b/t/t9904-git-merge-signoff.sh > > The convention for adding new tests is not to add a new one after > whatever name sorts the highest, see "Naming Tests" in t/README. Correct. > I.e. this should be somewhere in t[6-7]* with the other merge tests. Yeah. While most of t[67]??? series are about the contents of the merge, i.e. resulting trees and what happens in the working tree, there are some tests about the merge messages in there. t7608 is exactly about how the command prepares the messages before giving them to human to edit, and I think "merge can be told to optionally add sign-off" fits there just fine. All existing tests there are only interested about the title, but that does not mean there must not be tests that care more than the title in the script. Also, as you suggest, these will become a lot shorter when the standard test helper shell functions are used. I do not think we necessarily want a brand new test script to test only three or so combinations (i.e. last one wins when --option and --no-option is given, --option has an effect, --no-option does not have an effect).
Re: [PATCH v2] add --signoff flag to `git-merge` command.
On Tue, Jul 04 2017, Łukasz Gryglicki jotted: > add --signoff flag to `git-merge` command. We'd usually say this as: merge: add a --signoff flag Or something like that. > Some projects require every commit to be signed off. > Our workflow is to create feature branches and require every commit to > be signed off. When feature is finally approved we need to merge it into > master. Merge itself is usually trivial and is done by > `git merge origin/master`. Unfortunatelly this command have no --signoff > flag, so we need to either add signoff line manually or use > `git commit --amend -s` after the merge. First solution is not ideal > because not all developers are familiar with exact sign-off syntax. > The second solution works, but is obviously tedious. > This patch adds --signoff support to git-merge command. It works just > like --signoff in `git-commit` command. It would be nice to split this into a at least a couple of paragraphs, and more closely follow the format suggested by Documentation/SubmittingPatches. > More details here: > https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-dy9h3kt4feymgv__p2gzznr0wua...@mail.gmail.com/T/#u These more details include my outstanding question in 87fueferd4@gmail.com which hasn't been answered yet. > Signed-off-by: lukaszgryglicki> --- > Documentation/git-merge.txt | 8 + > builtin/merge.c | 4 +++ > t/t9904-git-merge-signoff.sh | 75 > > > 3 files changed, 87 insertions(+) > create mode 100755 t/t9904-git-merge-signoff.sh > > diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt > index 04fdd8cf086db..6b308ab6d0b52 100644 > --- a/Documentation/git-merge.txt > +++ b/Documentation/git-merge.txt > @@ -64,6 +64,14 @@ OPTIONS > --- > include::merge-options.txt[] > > +--signoff:: > + Add Signed-off-by line by the committer at the end of the commit > + log message. The meaning of a signoff depends on the project, > + but it typically certifies that committer has > + the rights to submit this work under the same license and > + agrees to a Developer Certificate of Origin > + (see http://developercertificate.org/ for more information). > + > -S[]:: > --gpg-sign[=]:: > GPG-sign the resulting merge commit. The `keyid` argument is > diff --git a/builtin/merge.c b/builtin/merge.c > index 900bafdb45d0b..78c36e9bf353b 100644 > --- a/builtin/merge.c > +++ b/builtin/merge.c > @@ -70,6 +70,7 @@ static int continue_current_merge; > static int allow_unrelated_histories; > static int show_progress = -1; > static int default_to_upstream = 1; > +static int signoff; > static const char *sign_commit; > > static struct strategy all_strategy[] = { > @@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = { > { OPTION_STRING, 'S', "gpg-sign", _commit, N_("key-id"), > N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, > OPT_BOOL(0, "overwrite-ignore", _ignore, N_("update ignored > files (default)")), > + OPT_BOOL(0, "signoff", , N_("add Signed-off-by:")), > OPT_END() > }; > > @@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list > *remoteheads) > strbuf_addch(, '\n'); > if (0 < option_edit) > strbuf_commented_addf(, _(merge_editor_comment), > comment_line_char); > + if (signoff) > + append_signoff(, ignore_non_trailer(msg.buf, msg.len), 0); > write_file_buf(git_path_merge_msg(), msg.buf, msg.len); > if (run_commit_hook(0 < option_edit, get_index_file(), > "prepare-commit-msg", > git_path_merge_msg(), "merge", NULL)) > diff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh > new file mode 100755 > index 0..f542f136f5dda > --- /dev/null > +++ b/t/t9904-git-merge-signoff.sh The convention for adding new tests is not to add a new one after whatever name sorts the highest, see "Naming Tests" in t/README. I.e. this should be somewhere in t[6-7]* with the other merge tests. > @@ -0,0 +1,75 @@ > +#!/bin/sh > + > +test_description='git merge --signoff > + > +This test runs git merge --signoff and makes sure that it works. > +' > + > +. ./test-lib.sh > + > +# Setup test files > +test_setup() { > + # A simples files to commit > + echo "1" >file1 > + echo "2" >file2 > + echo "3" >file3 > + echo "4" >file4 > + > + # Expected commit message after merge --signoff > + cat >expected-signed < +Merge branch 'master' into other-branch > + > +Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/") > +EOF > + > + # Expected commit message after merge without --signoff (or with > --no-signoff) > + cat >expected-unsigned < +Merge branch 'master' into other-branch > +EOF > + > + # Initial commit and feature branch to merge master into it. > + git commit --allow-empty -m "Initial empty commit" > + git checkout
[PATCH v2] add --signoff flag to `git-merge` command.
Some projects require every commit to be signed off. Our workflow is to create feature branches and require every commit to be signed off. When feature is finally approved we need to merge it into master. Merge itself is usually trivial and is done by `git merge origin/master`. Unfortunatelly this command have no --signoff flag, so we need to either add signoff line manually or use `git commit --amend -s` after the merge. First solution is not ideal because not all developers are familiar with exact sign-off syntax. The second solution works, but is obviously tedious. This patch adds --signoff support to git-merge command. It works just like --signoff in `git-commit` command. More details here: https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-dy9h3kt4feymgv__p2gzznr0wua...@mail.gmail.com/T/#u Signed-off-by: lukaszgryglicki--- Documentation/git-merge.txt | 8 + builtin/merge.c | 4 +++ t/t9904-git-merge-signoff.sh | 75 3 files changed, 87 insertions(+) create mode 100755 t/t9904-git-merge-signoff.sh diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt index 04fdd8cf086db..6b308ab6d0b52 100644 --- a/Documentation/git-merge.txt +++ b/Documentation/git-merge.txt @@ -64,6 +64,14 @@ OPTIONS --- include::merge-options.txt[] +--signoff:: + Add Signed-off-by line by the committer at the end of the commit + log message. The meaning of a signoff depends on the project, + but it typically certifies that committer has + the rights to submit this work under the same license and + agrees to a Developer Certificate of Origin + (see http://developercertificate.org/ for more information). + -S[]:: --gpg-sign[=]:: GPG-sign the resulting merge commit. The `keyid` argument is diff --git a/builtin/merge.c b/builtin/merge.c index 900bafdb45d0b..78c36e9bf353b 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -70,6 +70,7 @@ static int continue_current_merge; static int allow_unrelated_histories; static int show_progress = -1; static int default_to_upstream = 1; +static int signoff; static const char *sign_commit; static struct strategy all_strategy[] = { @@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = { { OPTION_STRING, 'S', "gpg-sign", _commit, N_("key-id"), N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, OPT_BOOL(0, "overwrite-ignore", _ignore, N_("update ignored files (default)")), + OPT_BOOL(0, "signoff", , N_("add Signed-off-by:")), OPT_END() }; @@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list *remoteheads) strbuf_addch(, '\n'); if (0 < option_edit) strbuf_commented_addf(, _(merge_editor_comment), comment_line_char); + if (signoff) + append_signoff(, ignore_non_trailer(msg.buf, msg.len), 0); write_file_buf(git_path_merge_msg(), msg.buf, msg.len); if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg", git_path_merge_msg(), "merge", NULL)) diff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh new file mode 100755 index 0..f542f136f5dda --- /dev/null +++ b/t/t9904-git-merge-signoff.sh @@ -0,0 +1,75 @@ +#!/bin/sh + +test_description='git merge --signoff + +This test runs git merge --signoff and makes sure that it works. +' + +. ./test-lib.sh + +# Setup test files +test_setup() { + # A simples files to commit + echo "1" >file1 + echo "2" >file2 + echo "3" >file3 + echo "4" >file4 + + # Expected commit message after merge --signoff + cat >expected-signed <.*/>/") +EOF + + # Expected commit message after merge without --signoff (or with --no-signoff) + cat >expected-unsigned actual && + test_cmp expected-unsigned actual +' + +# Test for --no-signoff flag +test_expect_success 'git merge --no-signoff flag cancels --signoff flag' ' + git checkout master && + git add file4 && + git commit -m master-branch-3 && + git checkout other-branch && + git merge master --no-edit --signoff --no-signoff && + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual && + test_cmp expected-unsigned actual +' + +test_done -- https://github.com/git/git/pull/382