Re: Re : 2 questions on git-send-email usage

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: Re : 2 questions on git-send-email usage

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:33

Franck Bui-Huu [off-list ref] writes:
Maybe that patch does what you want.

-- >8 --

Subject: [PATCH] Add a newline before appending "Signed-off-by:"

It looks nicer.

Signed-off-by: Franck Bui-Huu <redacted>
Haven't checked the code around the patch yet, but does it work
when the original commit log message ends with a blank line and
existing signed-off-by lines by other people?  You do not want
an extra blank lines there.

Re: Re : 2 questions on git-send-email usage

From: Franck Bui-Huu <hidden>
Date: 2016-06-15 22:42:33

Junio C Hamano wrote:
Haven't checked the code around the patch yet, but does it work
when the original commit log message ends with a blank line and
existing signed-off-by lines by other people?  You do not want
an extra blank lines there.
argh, no I just tested the previous case. Here is an update
which fix all cases.

-- >8 --

[PATCH] Add a newline before appending "Signed-off-by:"

It looks nicer.

Signed-off-by: Franck Bui-Huu <redacted>
---
 log-tree.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 9d8d46f..69d5c8a 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -17,9 +17,10 @@ static int append_signoff(char *buf, int
 	int signoff_len = strlen(signoff);
 	static const char signed_off_by[] = "Signed-off-by: ";
 	char *cp = buf;
+	int has_signoff = 0;
 
 	/* Do we have enough space to add it? */
-	if (buf_sz - at <= strlen(signed_off_by) + signoff_len + 2)
+	if (buf_sz - at <= strlen(signed_off_by) + signoff_len + 3)
 		return at;
 
 	/* First see if we already have the sign-off by the signer */
@@ -32,8 +33,11 @@ static int append_signoff(char *buf, int
 		    !strncmp(cp, signoff, signoff_len) &&
 		    isspace(cp[signoff_len]))
 			return at; /* we already have him */
+		has_signoff = 1;
 	}
 
+	if (!has_signoff)
+		buf[at++] = '\n';
 	strcpy(buf + at, signed_off_by);
 	at += strlen(signed_off_by);
 	strcpy(buf + at, signoff);
-- 
1.4.1.g55b7

Re: Re : 2 questions on git-send-email usage

From: Linus Torvalds <torvalds@osdl.org>
Date: 2016-06-15 22:42:33


On Wed, 12 Jul 2006, Franck Bui-Huu wrote:
[PATCH] Add a newline before appending "Signed-off-by:"

It looks nicer.
Yes. However, I think the sign-off detection is a bit broken (quite 
independently of your patch).

A number of people end up capitalizing the sign-off differently, so you 
have lines like "Signed-Off-By: Xy Zzy [off-list ref]".

Also, at least for the kernel, we often have alternative formats, like

	Acked-by: Elliot Xavier Ample [off-list ref]

and for that case, adding the extra newline is actually bad.

So I would suggest a totally different approach: instead of using 
"strstr(comments, signed_off_by)", it would probably be much better to 
just look for the last non-empty line, and see if it matches the format

	"^[nonspace]*: .*@.*$"

(yeah, that's not a valid regexp, but you get the idea).

On a slightly related note, I absolutely _hate_ how cherry-picking adds 
"(cherry-picked from commit <sha1>)" at the end. It's wrong for so many 
reasons, one of them being that it then breaks things like this, but the 
main one being that <sha1> will quite often actually end up not even 
_existing_ in the resulting archive (you cherry-picked from your private 
branch, and even if you keep your branch, you don't necessarily push it 
out).

Junio, can we make the default _not_ to do it, please?

			Linus
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help