{"thread":{"id":"30618","subject":"fmt-merge-message: add empty line between tag and signature verification","startedAt":"2012-05-25T16:02:03Z","lastAt":"2012-05-25T18:35:30Z","messageCount":5,"participants":["Linus Torvalds","Junio C Hamano","Martin Fick","Ralf Thielow"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"192165","messageId":"alpine.LFD.2.02.1205250850001.19607@i5.linux-foundation.org","threadId":"30618","inReplyTo":null,"subject":"fmt-merge-message: add empty line between tag and signature verification","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-05-25T16:02:03Z","receivedAt":"2012-05-25T16:02:03Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nWhen adding the information from a tag, put an empty line between the \nmessage of the tag and the commented-out signature verification \ninformation.\n\nAt least for the kernel workflow, I often end up re-formatting the message \nthat people send me in the tag data. In that situation, putting the tag \nmessage and the tag signature verification back-to-back then means that \nnormal editor \"reflow parapgraph\" command will get confused and think that \nthe signature is a continuation of the last message paragraph.\n\nSo I always end up having to first add an empty line, and then go back and \nreflow the last paragraph. Let's just do it in git directly.\n\nThe extra vertical space also makes the verification visually stand out \nmore from the user-supplied message, so it looks a bit more readable to me \ntoo, but that may be just an odd personal preference.\n\nSigned-off-by: Linus Torvalds \n---\nThis is a throw-away patch - apply or not as you prefer. I thought I'd \nsend it out and see what people thought.\n\nI don't feel *that* strongly about it.\n\nBtw, I'd also like to see the merge notes (notably the conflict file list) \nbefore the generated shortlog, but that seems to really not work with the \ncurrent fmt-merge-message model. Oh well.\n\nAnd the strbuf_complete_line() change is entirely independent, but didn't \nseem worth an extra separate patch. Feel free to take that out, or do it \nindependently or whatever.\n\n builtin/fmt-merge-msg.c | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex a517f1794a1c..d42015d8672d 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -462,7 +462,10 @@ static void fmt_tag_signature(struct strbuf *tagbuf,\n \t\tstrbuf_add(tagbuf, tag_body, buf + len - tag_body);\n \t}\n \tstrbuf_complete_line(tagbuf);\n-\tstrbuf_add_lines(tagbuf, \"# \", sig->buf, sig->len);\n+\tif (sig->len) {\n+\t\tstrbuf_addch(tagbuf, '\\n');\n+\t\tstrbuf_add_lines(tagbuf, \"# \", sig->buf, sig->len);\n+\t}\n }\n \n static void fmt_merge_msg_sigs(struct strbuf *out)\n@@ -627,8 +630,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,\n \t\trev.ignore_merges = 1;\n \t\trev.limited = 1;\n \n-\t\tif (suffixcmp(out->buf, \"\\n\"))\n-\t\t\tstrbuf_addch(out, '\\n');\n+\t\tstrbuf_complete_line(out);\n \n \t\tfor (i = 0; i < origins.nr; i++)\n \t\t\tshortlog(origins.items[i].string,\n"},{"id":"192170","messageId":"7vtxz4tcmn.fsf@alter.siamese.dyndns.org","threadId":"30618","inReplyTo":"alpine.LFD.2.02.1205250850001.19607@i5.linux-foundation.org","subject":"Re: fmt-merge-message: add empty line between tag and signature verification","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-25T17:05:20Z","receivedAt":"2012-05-25T17:05:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> When adding the information from a tag, put an empty line between the \n> message of the tag and the commented-out signature verification \n> information.\n>\n> At least for the kernel workflow, I often end up re-formatting the message \n> that people send me in the tag data. In that situation, putting the tag \n> message and the tag signature verification back-to-back then means that \n> normal editor \"reflow parapgraph\" command will get confused and think that \n> the signature is a continuation of the last message paragraph.\n>\n> So I always end up having to first add an empty line, and then go back and \n> reflow the last paragraph. Let's just do it in git directly.\n>\n> The extra vertical space also makes the verification visually stand out \n> more from the user-supplied message, so it looks a bit more readable to me \n> too, but that may be just an odd personal preference.\n>\n> Signed-off-by: Linus Torvalds \n> ---\n> This is a throw-away patch - apply or not as you prefer. I thought I'd \n> send it out and see what people thought.\n>\n> I don't feel *that* strongly about it.\n\nSpeaking as one of \"people\" (as opposed to \"the maintainer to decide one\nway or the other in bikeshedding\"), I think this makes sense.\n\n> Btw, I'd also like to see the merge notes (notably the conflict file list) \n> before the generated shortlog, but that seems to really not work with the \n> current fmt-merge-message model. Oh well.\n\nPostponing to assess if such a change is feasible, I am not sure if that\nordering makes more sense than the current one. Is the objective to more\nstrongly motivate people to explain what happened to the conflicts?\n"},{"id":"192171","messageId":"CA+55aFwwUzErbcCGa597N4G7i-_mYso+aJO_0aRq0jNBOi9=ew@mail.gmail.com","threadId":"30618","inReplyTo":"7vtxz4tcmn.fsf@alter.siamese.dyndns.org","subject":"Re: fmt-merge-message: add empty line between tag and signature verification","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2012-05-25T17:20:10Z","receivedAt":"2012-05-25T17:20:10Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Fri, May 25, 2012 at 10:05 AM, Junio C Hamano wrote:\n>\n>> Btw, I'd also like to see the merge notes (notably the conflict file list)\n>> before the generated shortlog, but that seems to really not work with the\n>> current fmt-merge-message model.  Oh well.\n>\n> Postponing to assess if such a change is feasible, I am not sure if that\n> ordering makes more sense than the current one.  Is the objective to more\n> strongly motivate people to explain what happened to the conflicts?\n\nYes. Right now, the conflict line is hidden way at the bottom, and\nI've actually overlooked it several times (and I don't think I'm the\nonly one - do \"git log --grep=Conflicts:\" on the kernel tree and\nyou'll see a lot of people just leaving them be).\n\nAnd even when I *don't* overlook it, it's actually annoying to go to\nthem and edit them. They're not at the very end (that's the big\ncomment about merging), and due to the shortlog the conflicts are\nusually listed at least one screenful down.\n\nAnd if you *don't* edit them, they end up pretty ugly and useless. I\nlike to at the very least make them a bit more compact and readable,\nie something like\n\n Fix up trivial conflicts in arch/arm/mach-ux500/{Makefile,board-mop500.c}\n\ninstead of listing them individually, but preferably actually talk\nabout what the conflicts were about, eg\n\n Fix up add-add onflicts in arch/x86/xen/enlighten.c due to addition of\n apic ipi interface next to the new apic_id functions.\n\nwhich I think makes it more worthwhile to talk about the conflicts.\n\nIn fact, I think that if you *don't* edit them at all, you might as\nwell just leave the conflict data out entirely, and it might be worth\nmaking the automatic conflict information be commented by default. But\nthen it *really* needs to be above the shortlog so that it's not\nhidden away.\n\nThat said, this is all \"minor tweaking\". The current thing certainly\nworks. I just think it could be better. Clearly a lot of people do not\nedit the conflict information even when it is right there (see the\nsame \"git log --grep\" above), and it looks like I'm sadly alone in\ntrying to make good merge messages.\n\n Linus\n"},{"id":"192183","messageId":"201205251206.30783.mfick@codeaurora.org","threadId":"30618","inReplyTo":"CA+55aFwwUzErbcCGa597N4G7i-_mYso+aJO_0aRq0jNBOi9=ew@mail.gmail.com","subject":"Re: fmt-merge-message: add empty line between tag and signature verification","fromName":"Martin Fick","fromEmail":"mfick@codeaurora.org","sentAt":"2012-05-25T18:06:30Z","receivedAt":"2012-05-25T18:06:30Z","isPatch":false,"sender":{"key":"mfick@codeaurora.org","avatar":null},"body":"On Friday, May 25, 2012 11:20:10 am Linus Torvalds wrote:\n> On Fri, May 25, 2012 at 10:05 AM, Junio C Hamano \n wrote:\n> >> Btw, I'd also like to see the merge notes (notably the\n> >> conflict file list) before the generated shortlog,\n> >> but that seems to really not work with the current\n> >> fmt-merge-message model. Oh well.\n> > \n> Yes. Right now, the conflict line is hidden way at the\n> bottom, and I've actually overlooked it several times\n> (and I don't think I'm the only one - do \"git log\n> --grep=Conflicts:\" on the kernel tree and you'll see a\n> lot of people just leaving them be).\n\nTangentially related, I think that in the cherry-pick case \nthis can also be a problem. Not exactly the same, but in the \ncherry-pick case you are generally starting with an existing \ncommit message, and the location of the conflicts at the \nbottom ends up actually causing functional problems for the \ninterpretations of the old \"footer-lines\" by some tools. \n\nNotably, our users sometimes cherry-pick gerrit changes but \nthen the old Change-Id footer line gets placed above the \nconflicts and if they forget to remove the conflicts, the \nChange-Id footer is not recognized by Gerrit. \n\nSo, not only are users forgetting to remove the conflicts, \nit causes tooling problems in the cherry-pick case by \nmessing with the footer layouts,\n\n-Martin\n\n-- \nEmployee of Qualcomm Innovation Center, Inc. which is a \nmember of Code Aurora Forum\n"},{"id":"192189","messageId":"CAN0XMO+LH9ZYSAtRVLOPU3p0bP4mryHkML2wi1KhNEui+AL=uA@mail.gmail.com","threadId":"30618","inReplyTo":"7vtxz4tcmn.fsf@alter.siamese.dyndns.org","subject":"Re: fmt-merge-message: add empty line between tag and signature verification","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@googlemail.com","sentAt":"2012-05-25T18:35:30Z","receivedAt":"2012-05-25T18:35:30Z","isPatch":false,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"On Fri, May 25, 2012 at 7:05 PM, Junio C Hamano wrote:\n>> Btw, I'd also like to see the merge notes (notably the conflict file list)\n>> before the generated shortlog, but that seems to really not work with the\n>> current fmt-merge-message model.  Oh well.\n>\n> Postponing to assess if such a change is feasible, I am not sure if that\n> ordering makes more sense than the current one.  Is the objective to more\n> strongly motivate people to explain what happened to the conflicts?\n\nIMHO it only makes sense when you have a short list of conflicted files.\nI mostly work on Java projects and when we merge a branch this list can\nbe very long. This is caused by several tools, different IDEs, different OSes\nand so on and so forth. Anyway...\nWhat I want to see on a merge like this is the shortlog. I only become\ninterested in\nthe conflicts if there is an issue in the software caused by a\nconflict resolution.\nJust my experience on Java projects I'm working on.\n"}]}