threads / patch / 25762

patchcherry-pick -x: add newline before pick note

Subject: [PATCH] cherry-pick -x: add newline before pick note

## tl;dr

13 messages between Nov 16, 2010 and Mar 8, 2011. Diffs are folded; open one to read it.

replies: 12people: 6as markdown or json

Michael J Gruber· Nov 16, 2010, 15:11 UTC · lore

Currently, cherry-pick -x sticks the pick note immediately after the existing commit message. This

* is bad for commits with 1 line subject (it makes a 2 line subject)
* is different from git-svn, e.g., which leaves an empty line before.
Make cherry-pick always insert an empty line before the pick note.
Reported-by: Martin Svensson <martin.k.svensson@netinsight.se>
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 builtin/revert.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to builtin/revert.c +1 −1
diff --git a/builtin/revert.c b/builtin/revert.c
index 57b51e4..9251257 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -485,7 +485,7 @@ static int do_pick_commit(void)
 		set_author_ident_env(msg.message);
 		add_message_to_msg(&msgbuf, msg.message);
 		if (no_replay) {
-			strbuf_addstr(&msgbuf, "(cherry picked from commit ");
+			strbuf_addstr(&msgbuf, "\n(cherry picked from commit ");
 			strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
 			strbuf_addstr(&msgbuf, ")\n");
 		}
-- 
1.7.3.2.193.g78bbb
Jeff King· Nov 16, 2010, 19:30 UTC · re: Michael J Gruber · lore

Re: [PATCH] cherry-pick -x: add newline before pick note

On Tue, Nov 16, 2010 at 04:11:17PM +0100, Michael J Gruber wrote:
Show 7 quoted lines
> Currently, cherry-pick -x sticks the pick note immediately after the
> existing commit message. This
> 
> * is bad for commits with 1 line subject (it makes a 2 line subject)
> * is different from git-svn, e.g., which leaves an empty line before.
> 
> Make cherry-pick always insert an empty line before the pick note.

Hmm. Should this respect pseudo-header blocks at the end? E.g., if I have:

  message subject
  Message body.
  Signed-off-by: Jeff King <peff@peff.net>
shouldn't it result in:
  message subject
  Message body.
  (cherry picked from commit ...)
  Signed-off-by: Jeff King <peff@peff.net>
?
Even better, I wonder if it should actually be:
  message subject
  Message body.
  Signed-off-by: Jeff King <peff@peff.net>
  Cherry-picked-from: ...

And then you could actually sign off the cherry-pick separately, too, if you wanted, by adding a line _below_ the cherry-picked-from. I have no idea if people are trying to grep for "cherry picked from commit...", which my proposal would break.

Note that none of this is introduced by your patch. The current output for this case is terribly ugly. But I thought I would mention it, as my third version means we _do_ want the current behavior in some cases (i.e., when there is already a pseudo-header block).

-Peff
Jonathan Nieder· Nov 16, 2010, 20:25 UTC · re: Jeff King · lore

[PATCH] commit -s: allow "(cherry picked " lines in sign-off section

Using cherry-pick -x -s to backport a public commit results in an unsightly gap in the sign-off chain:

	Reported-by: Jarek Poplawski <jarkao2@gmail.com>
	Tested-by: Jarek Poplawski <jarkao2@gmail.com>
	Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
	Cc: Jeff Mahoney <jeffm@suse.com>
	Cc: All since 2.6.32 <stable@kernel.org>
	Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
	Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
	(cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)
	Signed-off-by: Back Porter <backporter@example.com>

The cherry-pick is a step in the line of a patch like any other, so one might prefer to lose the extra newline.

	...
	Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
	(cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)
	Signed-off-by: Back Porter <backporter@example.com>

This commit teaches "git commit --signoff", and thus cherry-pick -s, to do exactly that. It works by treating the "(cherry picked" line as just another line in the signoff chain, except as the first line (that last exception is to avoid false positives).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jeff King wrote:
Show 8 quoted lines
> Even better, I wonder if it should actually be:
> 
>   message subject
> 
>   Message body.
> 
>   Signed-off-by: Jeff King <peff@peff.net>
>   Cherry-picked-from: ...

Here's something like that. I use "git cherry-pick -x -s" to backport patches from a public upstream. Now you can, too.

Ideally inline notes like
	 [akpm@linux-foundation.org: coding-style fixes]
also ought to be tolerated.
 builtin/commit.c               |   20 +++++++
 t/t3510-cherry-pick-message.sh |  112 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 132 insertions(+), 0 deletions(-)
 create mode 100755 t/t3510-cherry-pick-message.sh
Show changes to 2 files +132 −0

builtin/commit.c, t/t3510-cherry-pick-message.sh

diff --git a/builtin/commit.c b/builtin/commit.c
index 66fdd22..71dd52b 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -528,6 +528,8 @@ static int ends_rfc2822_footer(struct strbuf *sb)
 		i++;
 
 	for (; i < len; i = k) {
+		static const char cherry_pick[] = "(cherry picked from commit ";
+
 		for (k = i; k < len && buf[k] != '\n'; k++)
 			; /* do nothing */
 		k++;
@@ -535,6 +537,20 @@ static int ends_rfc2822_footer(struct strbuf *sb)
 		if ((buf[k] == ' ' || buf[k] == '\t') && !first)
 			continue;
 
+		if (!first && buf[k] == '(' && k + strlen(cherry_pick) < len) {
+			/* Might be a cherry-pick notice. */
+			const char *p = buf + k;
+			if (!memcmp(p, cherry_pick, strlen(cherry_pick))) {
+				p = memchr(buf + k, '\n', len - k);
+				if (!p)
+					return 0;
+				if (p + 1 == buf + len)
+					return 1;
+				k = p - buf;
+				continue;
+			}
+		}
+
 		first = 0;
 
 		for (j = 0; i + j < len; j++) {
@@ -625,6 +641,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
 			; /* do nothing */
 		if (prefixcmp(sb.buf + i, sob.buf)) {
+			/*
+			 * Only insert an extra newline if the previous line
+			 * is not part of a Signed-off-by:/Acked-by:/etc chain.
+			 */
 			if (!i || !ends_rfc2822_footer(&sb))
 				strbuf_addch(&sb, '\n');
 			strbuf_addbuf(&sb, &sob);
diff --git a/t/t3510-cherry-pick-message.sh b/t/t3510-cherry-pick-message.sh
new file mode 100755
index 0000000..83e50a3
--- /dev/null
+++ b/t/t3510-cherry-pick-message.sh
@@ -0,0 +1,112 @@
+#!/bin/sh
+
+test_description='tests for the log messages cherry-pick produces
+
+  ----
+  +      cherry-pick of branch
+     +   signoff
+    +    basic
+  ++     mainline
+  +++    initial
+'
+. ./test-lib.sh
+
+prepare_commit () {
+	git checkout initial &&
+	test_commit "$1" &&
+	test_tick &&
+	git commit --amend --allow-empty-message -F "$1.message" &&
+	git tag -d "$1" &&
+	git tag "$1"
+}
+
+test_cmp_message () {
+	expect=$1 &&
+	shift &&
+	git log -1 --pretty=format:%B "$@" >actual &&
+	test_cmp "$expect" actual
+}
+
+cat >basic.message <<\EOF
+ A branch
+
+Here comes the lovely description of a change to pick up.
+Contributions come from many people:
+EOF
+
+{
+	cat basic.message
+	cat <<-\EOF
+
+	Signed-off-by: Foo <foo@example.com>
+	Signed-off-by: Bar <bar@example.com>
+	Tested-by: Baz <baz@example.com>
+	EOF
+} >signoff.message
+
+test_expect_success 'setup' '
+	test_commit initial &&
+	prepare_commit basic &&
+	prepare_commit signoff
+'
+
+test_expect_success 'cherry-pick preserves message' '
+	cat basic.message >expect &&
+	git checkout initial &&
+	git cherry-pick basic &&
+	test_cmp_message expect
+'
+
+test_expect_success 'cherry-pick -s adds signoff' '
+	{
+		cat basic.message &&
+		echo &&
+		echo "Signed-off-by: C O Mitter <committer@example.com>"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -s basic &&
+	test_cmp_message expect HEAD
+'
+
+test_expect_success 'cherry-pick -s integrates into existing signoff chain' '
+	{
+		cat signoff.message &&
+		echo "Signed-off-by: C O Mitter <committer@example.com>"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -s signoff &&
+	test_cmp_message expect HEAD
+'
+
+test_expect_success 'cherry-pick -x adds old commit id' '
+	{
+		cat basic.message &&
+		echo "(cherry picked from commit $(git rev-parse basic^0))"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -x basic &&
+	test_cmp_message expect HEAD
+'
+
+test_expect_success 'cherry-pick -x integrates into signoff chain' '
+	{
+		cat signoff.message &&
+		echo "(cherry picked from commit $(git rev-parse signoff^0))"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -x signoff &&
+	test_cmp_message expect HEAD
+'
+
+test_expect_success 'cherry-pick -x -s' '
+	{
+		cat signoff.message &&
+		echo "(cherry picked from commit $(git rev-parse signoff^0))"
+		echo "Signed-off-by: C O Mitter <committer@example.com>"
+	} >expect &&
+	git checkout initial &&
+	git cherry-pick -x -s signoff &&
+	test_cmp_message expect HEAD
+'
+
+test_done
-- 
1.7.2.3.551.g13682.dirty
Jonathan Nieder· Nov 16, 2010, 20:40 UTC · re: Jonathan Nieder · lore

Re: [PATCH] commit -s: allow "(cherry picked " lines in sign-off section

Jonathan Nieder wrote:
Show 6 quoted lines
> 	(cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)
> 
> 	Signed-off-by: Back Porter <backporter@example.com>
> 
> The cherry-pick is a step in the line of a patch like any other,
> so one might prefer to lose the extra newline.
Sigh.  s/line/life/
[...]
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Let's kick off the reviews.
Show 8 quoted lines
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -528,6 +528,8 @@ static int ends_rfc2822_footer(struct strbuf *sb)
>  		i++;
>  
>  	for (; i < len; i = k) {
> +		static const char cherry_pick[] = "(cherry picked from commit ";
> +
Better to share this string with builtin/revert.c, no?

What would happen when "(cherry picked ..." gets translated? Should only the current language's version be tolerated in the commit footer, or is there something more generic to match for that could take care of wording changes automatically?

Show 9 quoted lines
> @@ -535,6 +537,20 @@ static int ends_rfc2822_footer(struct strbuf *sb)
>  		if ((buf[k] == ' ' || buf[k] == '\t') && !first)
>  			continue;
>  
> +		if (!first && buf[k] == '(' && k + strlen(cherry_pick) < len) {
> +			/* Might be a cherry-pick notice. */
> +			const char *p = buf + k;
> +			if (!memcmp(p, cherry_pick, strlen(cherry_pick))) {
> +				p = memchr(buf + k, '\n', len - k);
Maybe simpler:
	p = memchr(...
	if (!p)
		return 0;
	i = p - buf;
to reuse the termination condition in the sign-off parser.

Presumably the main loop could use memchr() instead of open-coding it as well.

Junio C Hamano· Nov 16, 2010, 22:52 UTC · re: Jonathan Nieder · lore

Re: [PATCH] commit -s: allow "(cherry picked " lines in sign-off section

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 31 quoted lines
> Jonathan Nieder wrote:
>
>> 	(cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)
>> 
>> 	Signed-off-by: Back Porter <backporter@example.com>
>> 
>> The cherry-pick is a step in the line of a patch like any other,
>> so one might prefer to lose the extra newline.
>
> Sigh.  s/line/life/
>
> [...]
>> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
>
> Let's kick off the reviews.
>
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -528,6 +528,8 @@ static int ends_rfc2822_footer(struct strbuf *sb)
>>  		i++;
>>  
>>  	for (; i < len; i = k) {
>> +		static const char cherry_pick[] = "(cherry picked from commit ";
>> +
>
> Better to share this string with builtin/revert.c, no?
>
> What would happen when "(cherry picked ..." gets translated?
> Should only the current language's version be tolerated in
> the commit footer, or is there something more generic to
> match for that could take care of wording changes automatically?

With this patch you are declaring that "(cherry picked from..." is a magic marker just like "Signed-off-by: " never to be translated, no?

I am not sure I agree with the reasoning of this patch, by the way. A cherry-pick is an event that breaks the life of the patch, so it may even be a sensible thing to do to express "the above sign-off chain shows who were involved in the original commit; I am cherry-picking it out of context, and these people do not have much to do with the result" with a blank line on both sides of the "cherry picked" line, like this:

        A concise summary of the change
	A detailed description of the change, why it is needed, what
        was broken and why applying this is the best course of action.
	Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
	Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
	(cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)
	Signed-off-by: Back Porter <backporter@example.com>
Jonathan Nieder· Nov 16, 2010, 23:36 UTC · re: Junio C Hamano · lore

Re: [PATCH] commit -s: allow "(cherry picked " lines in sign-off section

Junio C Hamano wrote:
Show 18 quoted lines
> I am not sure I agree with the reasoning of this patch, by the way.  A
> cherry-pick is an event that breaks the life of the patch, so it may even
> be a sensible thing to do to express "the above sign-off chain shows who
> were involved in the original commit; I am cherry-picking it out of
> context, and these people do not have much to do with the result" with a
> blank line on both sides of the "cherry picked" line, like this:
> 
>	A concise summary of the change
> 
> 	A detailed description of the change, why it is needed, what
>	was broken and why applying this is the best course of action.
> 
> 	Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> 	Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> 
> 	(cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)
> 
> 	Signed-off-by: Back Porter <backporter@example.com>

How is the cherry-pick event different from the send-by-mail-and-apply event?

In both cases, the result has a distinct commit id and distinct signoff and it is unlikely that the previous patch handler was testing with the same tree as the next one. (And each patch handler should add relevant comments if the new situation warrants that.)

Junio C Hamano· Nov 17, 2010, 16:46 UTC · re: Jonathan Nieder · lore

Re: [PATCH] commit -s: allow "(cherry picked " lines in sign-off section

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 7 quoted lines
> How is the cherry-pick event different from the send-by-mail-and-apply
> event?
>
> In both cases, the result has a distinct commit id and distinct
> signoff and it is unlikely that the previous patch handler was testing
> with the same tree as the next one.  (And each patch handler should add
> relevant comments if the new situation warrants that.)

Fair enough. If that is the direction we would want to go, perhaps it suggests that we might eventually want to use "Cherry-picked-from: " as the marker for this information?

And if we go that route, and if this information is being used, we have a rather serious backward compatibility problem. Older scripts will break, and this cannot be handwaved away with a configuration option or a command line switch (even if you personally choose to keep using the old format, you may get a commit with the new style trailer from elsewhere).

Hmm.
Jay Soffian· Nov 17, 2010, 06:23 UTC · re: Junio C Hamano · lore

Re: [PATCH] commit -s: allow "(cherry picked " lines in sign-off section

On Tue, Nov 16, 2010 at 5:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 18 quoted lines
> I am not sure I agree with the reasoning of this patch, by the way.  A
> cherry-pick is an event that breaks the life of the patch, so it may even
> be a sensible thing to do to express "the above sign-off chain shows who
> were involved in the original commit; I am cherry-picking it out of
> context, and these people do not have much to do with the result" with a
> blank line on both sides of the "cherry picked" line, like this:
>
>        A concise summary of the change
>
>        A detailed description of the change, why it is needed, what
>        was broken and why applying this is the best course of action.
>
>        Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>        Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>
>        (cherry picked from commit 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0)
>
>        Signed-off-by: Back Porter <backporter@example.com>

Or perhaps prefix them with Original-, inspired by email headers, and which I think makes it even more clear that the sob lines don't apply to the new commit.

        Original-Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
        Original-Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
        Cherry-picked-from: 9d8117e72bf453dd9d85e0cd322ce4a0f8bccbc0
        Signed-off-by: Back Porter <backporter@example.com>
j.
Jay Soffian· Nov 17, 2010, 06:14 UTC · re: Jeff King · lore

Re: [PATCH] cherry-pick -x: add newline before pick note

On Tue, Nov 16, 2010 at 2:30 PM, Jeff King <peff@peff.net> wrote:
Show 9 quoted lines
> shouldn't it result in:
>
>  message subject
>
>  Message body.
>
>  (cherry picked from commit ...)
>
>  Signed-off-by: Jeff King <peff@peff.net>
+1.
Show 8 quoted lines
> Even better, I wonder if it should actually be:
>
>  message subject
>
>  Message body.
>
>  Signed-off-by: Jeff King <peff@peff.net>
>  Cherry-picked-from: ...
+2.
> And then you could actually sign off the cherry-pick separately, too, if
> you wanted, by adding a line _below_ the cherry-picked-from. I have no
> idea if people are trying to grep for "cherry picked from commit...",
> which my proposal would break.

I can fix my regex easily enough, but I'd also be happy to have this use some other switch than -x.

BTW, I notice that cherry-pick also misbehaves if the original commit message doesn't end in a newline. I'm not sure whether that's a cherry-pick bug for not checking that case, or whether it's a commit bug for not ensuring a newline terminates the commit message.

j.
Oswald Buddenhagen· Mar 8, 2011, 12:54 UTC · re: Michael J Gruber · lore

Re: [PATCH] cherry-pick -x: add newline before pick note

Michael J Gruber <git <at> drmicha.warpmail.net> writes:
Show 27 quoted lines
> Currently, cherry-pick -x sticks the pick note immediately after the
> existing commit message. This
> 
> * is bad for commits with 1 line subject (it makes a 2 line subject)
> * is different from git-svn, e.g., which leaves an empty line before.
> 
> Make cherry-pick always insert an empty line before the pick note.
> 
> Reported-by: Martin Svensson <martin.k.svensson <at> netinsight.se>
> Signed-off-by: Michael J Gruber <git <at> drmicha.warpmail.net>
> ---
>  builtin/revert.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/builtin/revert.c b/builtin/revert.c
> index 57b51e4..9251257 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -485,7 +485,7 @@ static int do_pick_commit(void)
>  		set_author_ident_env(msg.message);
>  		add_message_to_msg(&msgbuf, msg.message);
>  		if (no_replay) {
> -			strbuf_addstr(&msgbuf, "(cherry picked from commit ");
> +			strbuf_addstr(&msgbuf, "\n(cherry picked from commit ");
>  			strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
>  			strbuf_addstr(&msgbuf, ")\n");
>  		}

so while everybody is apparently thinking about totally over-engineering things as much as possible, could we please have this patch applied so we have a solution for the time being? i really hate to tell my coworkers that they have to amend the cherry-picks just to make them comply with git's own guidelines for well-formed commit messages (and thus have them pass our pre-receive hook).

regards
Jonathan Nieder· Mar 8, 2011, 22:08 UTC · re: Oswald Buddenhagen · lore

Re: [PATCH] cherry-pick -x: add newline before pick note

(please do not cull the CC list) Hi Oswald,

Oswald Buddenhagen wrote:
> so while everybody is apparently thinking about totally over-engineering
> things as much as possible, could we please have this patch applied so we
> have a solution for the time being?

I am not convinced that this patch makes a positive change in general. Starting from a message

	Foo the bar
	Make some excellent improvement to the frobnicator.
	Signed-off-by: A U Thor <author@example.com>
a person passing on the patch might write
	Foo the bar
[...]
	Signed-off-by: A U Thor <author@example.com>
	[committer@example.com: avoid multiple return points]
	Signed-off-by: C O Mitter <committer@example.com>

Similarly, when cherry-picking from permanent history, it can make sense to write

	Foo the bar
[...]
	Signed-off-by: A U Thor <author@example.com>
	(cherry picked from commit 78a8b989a76c8798a9898c98a98c98a98ca)
	Signed-off-by: C O Mitter <committer@example.com>

In both cases, it's just another hop in the life of a patch and not something that seems to deserve emphasis with extra whitespace.

> i really hate to tell my coworkers that
> they have to amend the cherry-picks just to make them comply with git's
> own guidelines for well-formed commit messages (and thus have them pass
> our pre-receive hook).

I assume you are referring to one-line commit messages becoming two-line? Here's something rough to start.

Show changes to builtin/revert.c +23 −0
diff --git a/builtin/revert.c b/builtin/revert.c
index dc1b702..343e7e7 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -198,6 +198,28 @@ static void add_message_to_msg(struct strbuf *msgbuf, const char *message)
 	strbuf_addstr(msgbuf, p);
 }
 
+static void add_blank_line_if_oneline(struct strbuf *msgbuf)
+{
+	const char *newline = memchr(msgbuf->buf, '\n', msgbuf->len);
+
+	if (!newline) {	/* No newline at end of message. */
+		strbuf_addch(msgbuf, '\n');
+		newline = msgbuf->buf + msgbuf->len - 1;
+	}
+
+	/*
+	 * If the change description consists of a single line,
+	 * add a blank line separating the title from the new
+	 * message body (the "(cherry picked from" line).
+	 *
+	 * NEEDSWORK: it would be better to reuse the append_signoff
+	 * logic.
+	 */
+	newline = memchr(newline, '\n', msgbuf-> buf + msgbuf->len - newline);
+	if (!newline)
+		strbuf_addch(msgbuf, '\n');
+}
+
 static void set_author_ident_env(const char *message)
 {
 	const char *p = message;
@@ -499,6 +521,7 @@ static int do_pick_commit(void)
 		next_label = msg.label;
 		set_author_ident_env(msg.message);
 		add_message_to_msg(&msgbuf, msg.message);
+		add_blank_line_if_oneline(&msgbuf);
 		if (no_replay) {
 			strbuf_addstr(&msgbuf, "(cherry picked from commit ");
 			strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
Oswald Buddenhagen· Mar 8, 2011, 22:18 UTC · re: Jonathan Nieder · lore

Re: [PATCH] cherry-pick -x: add newline before pick note

On Tue, Mar 08, 2011 at 04:08:43PM -0600, Jonathan Nieder wrote:
> (please do not cull the CC list)
>
i posted via the gmane webform ...
Show 8 quoted lines
> Oswald Buddenhagen wrote:
> > i really hate to tell my coworkers that they have to amend the
> > cherry-picks just to make them comply with git's own guidelines for
> > well-formed commit messages
> 
> I assume you are referring to one-line commit messages becoming
> two-line?
>
yes
> Here's something rough to start.
> 

i did a much simpler patch in that vein as well, but scrapped it again - totally overengineered. the idea is to optimize the simple case - cherry-pick -x, push. everything else needs amends anyway.

regards
Jonathan Nieder· Mar 8, 2011, 22:34 UTC · re: Oswald Buddenhagen · lore

Re: [PATCH] cherry-pick -x: add newline before pick note

Hi again.
Oswald Buddenhagen wrote:
> i posted via the gmane webform ...

(See <http://thread.gmane.org/gmane.comp.version-control.git/154490>. It's obnoxious that there is not an easier way, I agree.)

> i did a much simpler patch in that vein as well, but scrapped it again -
> totally overengineered. the idea is to optimize the simple case -
> cherry-pick -x, push. everything else needs amends anyway.

It's been a vague wish of mine for a while to fix "cherry-pick -x -s". I currently use it without amends and tolerate the blank line between the "cherry picked" and the sign-off.

Maybe you can convince people that wanting no extra blank line between an existing sign-off and the "cherry picked from" line is overengineering and worth regressing. I am not a fanatic about it --- I just thought it was worth mentioning that this proposed change would not be seen as positive by everyone.

Put another way, I don't find the two words "totally overengineered" very convincing here. For what it's worth.

Cheers, Jonathan

← back to recent threads