Re: [PATCH v2] add --signoff flag to `git-merge` command.
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jul 4, 2017, 08:33 UTC
- Message-ID
- <87tw2sbnyl.fsf@gmail.com>
- In-Reply-To
- <0102015d0c41db74-34ff8613-05e2-44bb-a05b-2624108c36c8-000000@eu-west-1.amazonses.com>
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.
Show 11 quoted lines
> 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__p2gZzNr0WUAPUw@mail.gmail.com/T/#u
These more details include my outstanding question in 87fueferd4.fsf@gmail.com which hasn't been answered yet.
Show 62 quoted lines
> Signed-off-by: lukaszgryglicki <lukaszgryglicki@o2.pl>
> ---
> 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[<keyid>]::
> --gpg-sign[=<keyid>]::
> 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", &sign_commit, N_("key-id"),
> N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
> OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
> + OPT_BOOL(0, "signoff", &signoff, N_("add Signed-off-by:")),
> OPT_END()
> };
>
> @@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)
> strbuf_addch(&msg, '\n');
> if (0 < option_edit)
> strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
> + if (signoff)
> + append_signoff(&msg, 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 0000000000000..f542f136f5dda
> --- /dev/null
> +++ b/t/t9904-git-merge-signoff.shThe 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.
Show 35 quoted lines
> @@ -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
> +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 <<EOF
> +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 -b other-branch
> + git add file1
> + git commit -m other-branchThis setup function doesn't && its commands together, so a fail in any one of these won't be detecten. Any reason you don't just add this to the test_expect_success below, where we would detect this?
Also most of your commit/add etc. could probably be replaced with the test_commit helper, see t/README.