threads / patch / 20493

patchmerge: indicate remote tracking branches in merge message

Subject: [PATCH] merge: indicate remote tracking branches in merge message

## tl;dr

12 messages between Aug 9, 2009 and Aug 9, 2009. Diffs are folded; open one to read it.

replies: 11people: 2as markdown or json

Jeff King· Aug 9, 2009, 06:59 UTC · lore

Previously when merging directly from a local tracking branch like:

  git merge origin/master
The merge message said:
   Merge commit 'origin/master'
     * commit 'origin/master':
       ...
Instead, let's be more explicit about what we are merging:
   Merge remote branch 'origin/master'
     * origin/master:
       ...

We accomplish this by recognizing remote tracking branches in git-merge when we build the simulated FETCH_HEAD output that we feed to fmt-merge-msg.

Signed-off-by: Jeff King <peff@peff.net>
---
This is a repost of
  http://article.gmane.org/gmane.comp.version-control.git/119909

which got no response from you. I think it is a good idea, but I am not deeply committed to it. I mainly want a yes or no so I can clean it out of my patch queue.

 builtin-merge.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
Show changes to builtin-merge.c +11 −0
diff --git a/builtin-merge.c b/builtin-merge.c
index 0b12fb3..c5688e1 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -378,6 +378,17 @@ static void merge_name(const char *remote, struct strbuf *msg)
 		goto cleanup;
 	}
 
+	strbuf_setlen(&buf, 0);
+	strbuf_addstr(&buf, "refs/remotes/");
+	strbuf_addstr(&buf, remote);
+	resolve_ref(buf.buf, branch_head, 0, 0);
+
+	if (!hashcmp(remote_head->sha1, branch_head)) {
+		strbuf_addf(msg, "%s\t\tremote branch '%s' of .\n",
+				sha1_to_hex(branch_head), remote);
+		goto cleanup;
+	}
+
 	/* See if remote matches <name>^^^.. or <name>~<number> */
 	for (len = 0, ptr = remote + strlen(remote);
 	     remote < ptr && ptr[-1] == '^';

-- 
1.6.4.72.g50730
Jeff King· Aug 9, 2009, 07:14 UTC · re: Jeff King · lore

Re: [PATCH] merge: indicate remote tracking branches in merge message

On Sun, Aug 09, 2009 at 02:59:36AM -0400, Jeff King wrote:
Show 7 quoted lines
> This is a repost of
> 
>   http://article.gmane.org/gmane.comp.version-control.git/119909
> 
> which got no response from you. I think it is a good idea, but I am not
> deeply committed to it. I mainly want a yes or no so I can clean it out
> of my patch queue.

Ah, it may have had to do with the fact that it doesn't actually pass the tests. There is a trivial text update needed in t3409, which calls "git merge origin/master" during its setup phase.

Updated, passing-all-tests patch is below. Sorry for the confusion.
-- >8 --
Subject: [PATCH] merge: indicate remote tracking branches in merge message

Previously when merging directly from a local tracking branch like:

  git merge origin/master
The merge message said:
   Merge commit 'origin/master'
     * commit 'origin/master':
       ...
Instead, let's be more explicit about what we are merging:
   Merge remote branch 'origin/master'
     * origin/master:
       ...

We accomplish this by recognizing remote tracking branches in git-merge when we build the simulated FETCH_HEAD output that we feed to fmt-merge-msg.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-merge.c                   |   11 +++++++++++
 t/t3409-rebase-preserve-merges.sh |    2 +-
 2 files changed, 12 insertions(+), 1 deletions(-)
Show changes to 2 files +12 −1

builtin-merge.c, t/t3409-rebase-preserve-merges.sh

diff --git a/builtin-merge.c b/builtin-merge.c
index 82b5466..1f59567 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -378,6 +378,17 @@ static void merge_name(const char *remote, struct strbuf *msg)
 		goto cleanup;
 	}
 
+	strbuf_setlen(&buf, 0);
+	strbuf_addstr(&buf, "refs/remotes/");
+	strbuf_addstr(&buf, remote);
+	resolve_ref(buf.buf, branch_head, 0, 0);
+
+	if (!hashcmp(remote_head->sha1, branch_head)) {
+		strbuf_addf(msg, "%s\t\tremote branch '%s' of .\n",
+				sha1_to_hex(branch_head), remote);
+		goto cleanup;
+	}
+
 	/* See if remote matches <name>^^^.. or <name>~<number> */
 	for (len = 0, ptr = remote + strlen(remote);
 	     remote < ptr && ptr[-1] == '^';
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index e6c8327..2ce0236 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -71,7 +71,7 @@ test_expect_success 'rebase -p fakes interactive rebase' '
 	git fetch &&
 	git rebase -p origin/topic &&
 	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge commit" | wc -l)
+	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote branch" | wc -l)
 	)
 '
 

-- 
1.6.4.178.g7a987
Junio C Hamano· Aug 9, 2009, 07:31 UTC · re: Jeff King · lore

Re: [PATCH] merge: indicate remote tracking branches in merge message

Jeff King <peff@peff.net> writes:
Show 32 quoted lines
> Previously when merging directly from a local tracking
> branch like:
>
>   git merge origin/master
>
> The merge message said:
>
>    Merge commit 'origin/master'
>
>      * commit 'origin/master':
>        ...
>
> Instead, let's be more explicit about what we are merging:
>
>    Merge remote branch 'origin/master'
>
>      * origin/master:
>        ...
>
> We accomplish this by recognizing remote tracking branches
> in git-merge when we build the simulated FETCH_HEAD output
> that we feed to fmt-merge-msg.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This is a repost of
>
>   http://article.gmane.org/gmane.comp.version-control.git/119909
>
> which got no response from you. I think it is a good idea, but I am not
> deeply committed to it. I mainly want a yes or no so I can clean it out
> of my patch queue.

I somewhat suspect that the patch was not applied because it also lacked necessary adjustments to tests. With this patch, I think the tests would fail.

Nevertheless, I think it is a good thing to do. But I am unsure about the implementation.

Shouldn't it instead feed what it got from the end user to the dwim machinery, and make sure it dwims into refs/remotes/ hierarchy?

In other words, like this. Note that it would be much clearer to see what's needed, if you want to extend it to refs/tags hierarchy ;-)

 builtin-merge.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)
Show changes to builtin-merge.c +12 −8
diff --git a/builtin-merge.c b/builtin-merge.c
index 82b5466..f4de73f 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -358,6 +358,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf bname = STRBUF_INIT;
 	const char *ptr;
+	char *found_ref;
 	int len, early;
 
 	strbuf_branchname(&bname, remote);
@@ -368,14 +369,17 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	if (!remote_head)
 		die("'%s' does not point to a commit", remote);
 
-	strbuf_addstr(&buf, "refs/heads/");
-	strbuf_addstr(&buf, remote);
-	resolve_ref(buf.buf, branch_head, 0, NULL);
-
-	if (!hashcmp(remote_head->sha1, branch_head)) {
-		strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
-			sha1_to_hex(branch_head), remote);
-		goto cleanup;
+	if (dwim_ref(remote, strlen(remote), branch_head, &found_ref) > 0) {
+		if (!prefixcmp(found_ref, "refs/heads/")) {
+			strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
+				    sha1_to_hex(branch_head), remote);
+			goto cleanup;
+		}
+		if (!prefixcmp(found_ref, "refs/remotes/")) {
+			strbuf_addf(msg, "%s\t\tremote branch '%s' of .\n",
+				    sha1_to_hex(branch_head), remote);
+			goto cleanup;
+		}
 	}
 
 	/* See if remote matches <name>^^^.. or <name>~<number> */
Jeff King· Aug 9, 2009, 07:40 UTC · re: Junio C Hamano · lore

Re: [PATCH] merge: indicate remote tracking branches in merge message

On Sun, Aug 09, 2009 at 12:31:04AM -0700, Junio C Hamano wrote:
> I somewhat suspect that the patch was not applied because it also lacked
> necessary adjustments to tests.  With this patch, I think the tests would
> fail.
Yeah, see my follow-up patch.
Show 5 quoted lines
> Nevertheless, I think it is a good thing to do.  But I am unsure about the
> implementation.
> 
> Shouldn't it instead feed what it got from the end user to the dwim
> machinery, and make sure it dwims into refs/remotes/ hierarchy?

I'm not sure that is all that different in practice than what is happening now. Mainly I did it the way I did so that I didn't touch the code path for detecting local branches.

But assuming they are functionally identical, I think your patch is much more readable.

> In other words, like this.  Note that it would be much clearer to see
> what's needed, if you want to extend it to refs/tags hierarchy ;-)

I'm not sure adding "tag foo" will actually work, as it still has to make it through the bit where we parse FETCH_HEAD. I'm not sure if it would get mutilated to "commit foo" by that code or not.

-Peff
Jeff King· Aug 9, 2009, 09:14 UTC · re: Jeff King · lore

Re: [PATCH] merge: indicate remote tracking branches in merge message

On Sun, Aug 09, 2009 at 03:40:35AM -0400, Jeff King wrote:
Show 9 quoted lines
> > Shouldn't it instead feed what it got from the end user to the dwim
> > machinery, and make sure it dwims into refs/remotes/ hierarchy?
> 
> I'm not sure that is all that different in practice than what is
> happening now. Mainly I did it the way I did so that I didn't touch the
> code path for detecting local branches.
> 
> But assuming they are functionally identical, I think your patch is much
> more readable.

I tested your patch; the two methods aren't identical. In fact, yours fixes a bug. :)

In t4202, we have a branch name and a tag name that are the same (octopus-a), and we "git merge octopus-a". This actually merges the tag, but because the branch name existed, we write "Merge branch 'octopus-a'" in the log, which is not true. With your patch, it does the right thing and says "Merge commit 'octopus-a'".

The simple thing is to just update the "expect" text. Though the current behavior does show off the ability to collape the two branches and say

  Merge branches 'octopus-a' and 'octopus-b'
instead of
  Merge commit 'octopus-a'; commit 'octopus-b'
which we could preserve that by renaming the tags, as below.
---
Show changes to t/t4202-log.sh +6 −6
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 48e0088..9d0db2a 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -306,13 +306,13 @@ test_expect_success 'set up more tangled history' '
 	git checkout master &&
 	git merge tangle &&
 	git checkout -b reach &&
-	test_commit reach &&
+	test_commit reach-1 &&
 	git checkout master &&
 	git checkout -b octopus-a &&
-	test_commit octopus-a &&
+	test_commit octopus-a-1 &&
 	git checkout master &&
 	git checkout -b octopus-b &&
-	test_commit octopus-b &&
+	test_commit octopus-b-1 &&
 	git checkout master &&
 	test_commit seventh &&
 	git merge octopus-a octopus-b
@@ -327,12 +327,12 @@ cat > expect <<\EOF
 *-. \   Merge branches 'octopus-a' and 'octopus-b'
 |\ \ \
 * | | | seventh
-| | * | octopus-b
+| | * | octopus-b-1
 | |/ /
 |/| |
-| * | octopus-a
+| * | octopus-a-1
 |/ /
-| * reach
+| * reach-1
 |/
 *   Merge branch 'tangle'
 |\
Jeff King· Aug 9, 2009, 10:00 UTC · re: Jeff King · lore

Re: [PATCH] merge: indicate remote tracking branches in merge message

On Sun, Aug 09, 2009 at 05:14:43AM -0400, Jeff King wrote:
Show 14 quoted lines
> In t4202, we have a branch name and a tag name that are the same
> (octopus-a), and we "git merge octopus-a". This actually merges the tag,
> but because the branch name existed, we write "Merge branch 'octopus-a'"
> in the log, which is not true. With your patch, it does the right thing
> and says "Merge commit 'octopus-a'".
> 
> The simple thing is to just update the "expect" text. Though the current
> behavior does show off the ability to collape the two branches and say
> 
>   Merge branches 'octopus-a' and 'octopus-b'
> 
> instead of
> 
>   Merge commit 'octopus-a'; commit 'octopus-b'

Thinking about it for a few seconds, it's silly to try to test something that happens to occur in a totally unrelated test. The right thing to do is to write actual tests for this area, fix the bug, and then add the new feature. So how about this series:

  [1/3] add tests for merge message headings
  [2/3] merge: fix incorrect merge message for ambiguous tag/branch
  [3/3] merge: indicate remote tracking branches in merge message
-Peff
Jeff King· Aug 9, 2009, 10:01 UTC · re: Jeff King · lore

[PATCH 1/3] add tests for merge message headings

When calling "git merge $X", we automatically generate a commit message containing something like "Merge branch '$X'". This test script checks that those messages say what they should, and exposes a failure when merging a refname that is ambiguous between a tag and a branch.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t7608-merge-messages.sh |   50 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)
 create mode 100755 t/t7608-merge-messages.sh
Show changes to t/t7608-merge-messages.sh +50 −0
diff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh
new file mode 100755
index 0000000..9d10583
--- /dev/null
+++ b/t/t7608-merge-messages.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='test auto-generated merge messages'
+. ./test-lib.sh
+
+check_oneline() {
+	echo "$1" | sed "s/Q/'/g" >expect &&
+	git log -1 --pretty=tformat:%s >actual &&
+	test_cmp expect actual
+}
+
+test_expect_success 'merge local branch' '
+	test_commit master-1 &&
+	git checkout -b local-branch &&
+	test_commit branch-1 &&
+	git checkout master &&
+	test_commit master-2 &&
+	git merge local-branch &&
+	check_oneline "Merge branch Qlocal-branchQ"
+'
+
+test_expect_success 'merge octopus branches' '
+	git checkout -b octopus-a master &&
+	test_commit octopus-1 &&
+	git checkout -b octopus-b master &&
+	test_commit octopus-2 &&
+	git checkout master &&
+	git merge octopus-a octopus-b &&
+	check_oneline "Merge branches Qoctopus-aQ and Qoctopus-bQ"
+'
+
+test_expect_success 'merge tag' '
+	git checkout -b tag-branch master &&
+	test_commit tag-1 &&
+	git checkout master &&
+	test_commit master-3 &&
+	git merge tag-1 &&
+	check_oneline "Merge commit Qtag-1Q"
+'
+
+test_expect_failure 'ambiguous tag' '
+	git checkout -b ambiguous master &&
+	test_commit ambiguous &&
+	git checkout master &&
+	test_commit master-4 &&
+	git merge ambiguous &&
+	check_oneline "Merge commit QambiguousQ"
+'
+
+test_done

-- 
1.6.4.178.g7a987
Jeff King· Aug 9, 2009, 10:02 UTC · re: Jeff King · lore

[PATCH 2/3] merge: fix incorrect merge message for ambiguous tag/branch

If we have both a tag and a branch named "foo", then calling "git merge foo" will warn about the ambiguous ref, but merge the tag.

When generating the commit message, though, we simply checked whether "refs/heads/foo" existed, and if it did, assumed it was a branch. This led to the statement "Merge branch 'foo'" in the commit message, which is quite wrong.

Instead, we should use dwim_ref to find the actual ref used, and describe it appropriately.

In addition to the test in t7608, we must also tweak the expected output of t4202, which was accidentally triggering this bug.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-merge.c           |   15 +++++++--------
 t/t4202-log.sh            |    4 ++--
 t/t7608-merge-messages.sh |    2 +-
 3 files changed, 10 insertions(+), 11 deletions(-)
Show changes to 3 files +10 −11

builtin-merge.c, t/t4202-log.sh, t/t7608-merge-messages.sh

diff --git a/builtin-merge.c b/builtin-merge.c
index 82b5466..f7db148 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -358,6 +358,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf bname = STRBUF_INIT;
 	const char *ptr;
+	char *found_ref;
 	int len, early;
 
 	strbuf_branchname(&bname, remote);
@@ -368,14 +369,12 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	if (!remote_head)
 		die("'%s' does not point to a commit", remote);
 
-	strbuf_addstr(&buf, "refs/heads/");
-	strbuf_addstr(&buf, remote);
-	resolve_ref(buf.buf, branch_head, 0, NULL);
-
-	if (!hashcmp(remote_head->sha1, branch_head)) {
-		strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
-			sha1_to_hex(branch_head), remote);
-		goto cleanup;
+	if (dwim_ref(remote, strlen(remote), branch_head, &found_ref) > 0) {
+		if (!prefixcmp(found_ref, "refs/heads/")) {
+			strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
+				    sha1_to_hex(branch_head), remote);
+			goto cleanup;
+		}
 	}
 
 	/* See if remote matches <name>^^^.. or <name>~<number> */
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 48e0088..1e952ca 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -320,11 +320,11 @@ test_expect_success 'set up more tangled history' '
 '
 
 cat > expect <<\EOF
-*   Merge branch 'reach'
+*   Merge commit 'reach'
 |\
 | \
 |  \
-*-. \   Merge branches 'octopus-a' and 'octopus-b'
+*-. \   Merge commit 'octopus-a'; commit 'octopus-b'
 |\ \ \
 * | | | seventh
 | | * | octopus-b
diff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh
index 9d10583..81ced8a 100755
--- a/t/t7608-merge-messages.sh
+++ b/t/t7608-merge-messages.sh
@@ -38,7 +38,7 @@ test_expect_success 'merge tag' '
 	check_oneline "Merge commit Qtag-1Q"
 '
 
-test_expect_failure 'ambiguous tag' '
+test_expect_success 'ambiguous tag' '
 	git checkout -b ambiguous master &&
 	test_commit ambiguous &&
 	git checkout master &&

-- 
1.6.4.178.g7a987
Jeff King· Aug 9, 2009, 10:02 UTC · re: Jeff King · lore

[PATCH 3/3] merge: indicate remote tracking branches in merge message

Previously when merging directly from a local tracking branch like:

  git merge origin/master
The merge message said:
   Merge commit 'origin/master'
     * commit 'origin/master':
       ...
Instead, let's be more explicit about what we are merging:
   Merge remote branch 'origin/master'
     * origin/master:
       ...

We accomplish this by recognizing remote tracking branches in git-merge when we build the simulated FETCH_HEAD output that we feed to fmt-merge-msg.

In addition to a new test in t7608, we have to tweak the expected output of t3409, which does such a merge.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-merge.c                   |    5 +++++
 t/t3409-rebase-preserve-merges.sh |    2 +-
 t/t7608-merge-messages.sh         |   10 ++++++++++
 3 files changed, 16 insertions(+), 1 deletions(-)
Show changes to 3 files +16 −1

builtin-merge.c, t/t3409-rebase-preserve-merges.sh, t/t7608-merge-messages.sh

diff --git a/builtin-merge.c b/builtin-merge.c
index f7db148..f4de73f 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -375,6 +375,11 @@ static void merge_name(const char *remote, struct strbuf *msg)
 				    sha1_to_hex(branch_head), remote);
 			goto cleanup;
 		}
+		if (!prefixcmp(found_ref, "refs/remotes/")) {
+			strbuf_addf(msg, "%s\t\tremote branch '%s' of .\n",
+				    sha1_to_hex(branch_head), remote);
+			goto cleanup;
+		}
 	}
 
 	/* See if remote matches <name>^^^.. or <name>~<number> */
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index e6c8327..297d165 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -71,7 +71,7 @@ test_expect_success 'rebase -p fakes interactive rebase' '
 	git fetch &&
 	git rebase -p origin/topic &&
 	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge commit" | wc -l)
+	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote branch " | wc -l)
 	)
 '
 
diff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh
index 81ced8a..28d5679 100755
--- a/t/t7608-merge-messages.sh
+++ b/t/t7608-merge-messages.sh
@@ -47,4 +47,14 @@ test_expect_success 'ambiguous tag' '
 	check_oneline "Merge commit QambiguousQ"
 '
 
+test_expect_success 'remote branch' '
+	git checkout -b remote master &&
+	test_commit remote-1 &&
+	git update-ref refs/remotes/origin/master remote &&
+	git checkout master &&
+	test_commit master-5 &&
+	git merge origin/master &&
+	check_oneline "Merge remote branch Qorigin/masterQ"
+'
+
 test_done

-- 
1.6.4.178.g7a987
Jeff King· Aug 9, 2009, 10:07 UTC · re: Jeff King · lore

Re: [PATCH] merge: indicate remote tracking branches in merge message

On Sun, Aug 09, 2009 at 06:00:45AM -0400, Jeff King wrote:
>   [1/3] add tests for merge message headings
>   [2/3] merge: fix incorrect merge message for ambiguous tag/branch
>   [3/3] merge: indicate remote tracking branches in merge message
And here is the 4/3 you mentioned earlier:
-- >8 --
Subject: [PATCH] merge: describe tags as such in merge message

Previously, merging a tag directly via "git merge tag" would get you the message "Merge commit 'tag'". It is a little more descriptive to note that it was actually a tag (i.e., "Merge tag 'tag'").

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-merge.c           |    5 +++++
 t/t7608-merge-messages.sh |    4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)
Show changes to 2 files +7 −2

builtin-merge.c, t/t7608-merge-messages.sh

diff --git a/builtin-merge.c b/builtin-merge.c
index f4de73f..db74901 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -380,6 +380,11 @@ static void merge_name(const char *remote, struct strbuf *msg)
 				    sha1_to_hex(branch_head), remote);
 			goto cleanup;
 		}
+		if (!prefixcmp(found_ref, "refs/tags/")) {
+			strbuf_addf(msg, "%s\t\ttag '%s' of .\n",
+				    sha1_to_hex(branch_head), remote);
+			goto cleanup;
+		}
 	}
 
 	/* See if remote matches <name>^^^.. or <name>~<number> */
diff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh
index 28d5679..3ee0983 100755
--- a/t/t7608-merge-messages.sh
+++ b/t/t7608-merge-messages.sh
@@ -35,7 +35,7 @@ test_expect_success 'merge tag' '
 	git checkout master &&
 	test_commit master-3 &&
 	git merge tag-1 &&
-	check_oneline "Merge commit Qtag-1Q"
+	check_oneline "Merge tag Qtag-1Q"
 '
 
 test_expect_success 'ambiguous tag' '
@@ -44,7 +44,7 @@ test_expect_success 'ambiguous tag' '
 	git checkout master &&
 	test_commit master-4 &&
 	git merge ambiguous &&
-	check_oneline "Merge commit QambiguousQ"
+	check_oneline "Merge tag QambiguousQ"
 '
 
 test_expect_success 'remote branch' '

-- 
1.6.4.178.g7a987
Junio C Hamano· Aug 9, 2009, 19:36 UTC · re: Jeff King · lore

Re: [PATCH] merge: indicate remote tracking branches in merge message

Jeff King <peff@peff.net> writes:
Show 15 quoted lines
> On Sun, Aug 09, 2009 at 06:00:45AM -0400, Jeff King wrote:
>
>>   [1/3] add tests for merge message headings
>>   [2/3] merge: fix incorrect merge message for ambiguous tag/branch
>>   [3/3] merge: indicate remote tracking branches in merge message
>
> And here is the 4/3 you mentioned earlier:
>
> -- >8 --
> Subject: [PATCH] merge: describe tags as such in merge message
>
> Previously, merging a tag directly via "git merge tag" would
> get you the message "Merge commit 'tag'". It is a little
> more descriptive to note that it was actually a tag (i.e.,
> "Merge tag 'tag'").

Maybe "Merge version 'v1.6.3'" or "Merge commit tagged as 'v1.6.3'"? I dunno.

Jeff King· Aug 9, 2009, 21:49 UTC · re: Junio C Hamano · lore

Re: [PATCH] merge: indicate remote tracking branches in merge message

On Sun, Aug 09, 2009 at 12:36:17PM -0700, Junio C Hamano wrote:
Show 7 quoted lines
> > Previously, merging a tag directly via "git merge tag" would
> > get you the message "Merge commit 'tag'". It is a little
> > more descriptive to note that it was actually a tag (i.e.,
> > "Merge tag 'tag'").
> 
> Maybe "Merge version 'v1.6.3'" or "Merge commit tagged as 'v1.6.3'"? 
> I dunno.

Definitely "Merge version" is bad. Doing "git tag feature-x-working" and merging it would lead to "Merge version 'feature-x-working'", which doesn't really make sense. I don't think the developer would refer to such a tag as a "version".

Your "Merge commit tagged as..." version is reasonable to me. I'm dubious whether the additional information that it was a tag is actually worth anything. That is, given

  (1) Merge commit '1234abcd'
  (2) Merge commit 'v1.6.3'
  (3) Merge tag 'v1.6.3'

where (1) is what we do now and will do in the future for arbitrary commits, (2) is what we do now for tags, and (3) is the proposed change. I am not sure that (3) really tells the reader anything useful over the other two. And your "Merge commit tagged as" is just the same information written slightly differently.

So maybe this patch should simply be dropped. I don't feel strongly either way.

-Peff

← back to recent threads