{"thread":{"id":"46304","subject":"Why doesn't merge fail if message has only sign-off?","startedAt":"2017-07-02T12:03:14Z","lastAt":"2017-10-02T17:20:39Z","messageCount":24,"participants":["Kaartic Sivaraam","Junio C Hamano","Kevin Daudt","Christian Brabandt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"323743","messageId":"1498996988.26970.1.camel@gmail.com","threadId":"46304","inReplyTo":null,"subject":"Why doesn't merge fail if message has only sign-off?","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-07-02T12:03:08Z","receivedAt":"2017-07-02T12:03:14Z","isPatch":false,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"While trying to merge a branch using \"git merge\" if a merge\nmessage consists only of a \"Sign-off\" line it doesn't fail.\nTo be consistent with the behaviour of \"git commit\" shouldn't the merge\nfail?\n\n-- \nKaartic\n"},{"id":"323755","messageId":"xmqq60f9mo6b.fsf@gitster.mtv.corp.google.com","threadId":"46304","inReplyTo":"1498996988.26970.1.camel@gmail.com","subject":"Re: Why doesn't merge fail if message has only sign-off?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-03T17:21:32Z","receivedAt":"2017-07-03T17:21:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kaartic Sivaraam writes:\n\n> While trying to merge a branch using \"git merge\" if a merge\n> message consists only of a \"Sign-off\" line it doesn't fail.\n> To be consistent with the behaviour of \"git commit\" shouldn't the merge\n> fail?\n\nI think that it is not by design that it doesn't fail. It's not\nlike we decided to allow s-o-b only merge because we found a reason\nwhy it is a good idea to do so.\n\nSo I do not think anybody minds too deeply if somebody came up a\npatch to \"fix\" it. It's just that nobody tried to create such a\nsilly merge in real life so far (I do not think you did, either--you\nfound this out by playing around trying to find corner cases, no?)\n\n"},{"id":"323808","messageId":"1499198607.6428.9.camel@gmail.com","threadId":"46304","inReplyTo":"xmqq60f9mo6b.fsf@gitster.mtv.corp.google.com","subject":"Re: Why doesn't merge fail if message has only sign-off?","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-07-04T20:03:27Z","receivedAt":"2017-07-04T20:03:29Z","isPatch":false,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Mon, 2017-07-03 at 10:21 -0700, Junio C Hamano wrote:\n> I think that it is not by design that it doesn't fail.  It's not\n> like we decided to allow s-o-b only merge because we found a reason\n> why it is a good idea to do so.\n> \n> So I do not think anybody minds too deeply if somebody came up a\n> patch to \"fix\" it.  It's just that nobody tried to create such a\n> silly merge in real life so far (I do not think you did, either--you\n> found this out by playing around trying to find corner cases, no?)\n> \nYes and no. I found this out while playing around with the \"insert\nnotes in the commit template\" patch I sent previously. I wasn't trying\nto find corner cases, though.\n\n-- \nKaartic\n"},{"id":"323884","messageId":"20170706033149.6275-1-kaarticsivaraam91196@gmail.com","threadId":"46304","inReplyTo":"xmqq60f9mo6b.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] merge-message: change meaning of \"empty merge message\"","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-07-06T03:31:49Z","receivedAt":"2017-07-06T03:31:55Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"In the context of \"git merge\" the meaning of an \"empty message\"\nis one that contains no line of text. This is not in line with\n\"git commit\" where an \"empty message\" is one that contains only\nwhitespaces and/or signed-off-by lines. This could cause surprises\nto users who are accustomed to the meaning of an \"empty message\"\nof \"git commit\".\n\nPrevent such surprises by changing the meaning of an empty 'merge\nmessage' to be in line with that of an empty 'commit message'.\n\nSigned-off-by: Kaartic Sivaraam \n---\n builtin/merge.c | 35 ++++++++++++++++++++++++++++++++++-\n 1 file changed, 34 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 703827f00..db4bf1c40 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -748,6 +748,39 @@ static void abort_commit(struct commit_list *remoteheads, const char *err_msg)\n \texit(1);\n }\n \n+/*\n+ * Find out if the message in the strbuf contains only whitespace and\n+ * Signed-off-by lines.\n+ *\n+ * This function is the \"rest_is_space\" function of \"commit\" with the unwanted\n+ * parameter removed.\n+ */\n+static int message_is_empty(struct strbuf *sb)\n+{\n+\tint i, eol;\n+\tconst char *nl;\n+\n+\t/* Check if the rest is just whitespace and Signed-off-by's. */\n+\tfor (i = 0; i < sb->len; i++) {\n+\t\tnl = memchr(sb->buf + i, '\\n', sb->len - i);\n+\t\tif (nl)\n+\t\t\teol = nl - sb->buf;\n+\t\telse\n+\t\t\teol = sb->len;\n+\n+\t\tif (strlen(sign_off_header) <= eol - i &&\n+\t\t starts_with(sb->buf + i, sign_off_header)) {\n+\t\t\ti = eol;\n+\t\t\tcontinue;\n+\t\t}\n+\t\twhile (i < eol)\n+\t\t\tif (!isspace(sb->buf[i++]))\n+\t\t\t\treturn 0;\n+\t}\n+\n+\treturn 1;\n+}\n+\n static const char merge_editor_comment[] =\n N_(\"Please enter a commit message to explain why this merge is necessary,\\n\"\n \"especially if it merges an updated upstream into a topic branch.\\n\"\n@@ -772,7 +805,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t}\n \tread_merge_msg(&msg);\n \tstrbuf_stripspace(&msg, 0 < option_edit);\n-\tif (!msg.len)\n+\tif (!msg.len || message_is_empty(&msg))\n \t\tabort_commit(remoteheads, _(\"Empty commit message.\"));\n \tstrbuf_release(&merge_msg);\n \tstrbuf_addbuf(&merge_msg, &msg);\n-- \n2.11.0\n\n"},{"id":"323886","messageId":"20170706044640.GA11020@alpha.vpn.ikke.info","threadId":"46304","inReplyTo":"20170706033149.6275-1-kaarticsivaraam91196@gmail.com","subject":"Re: [PATCH] merge-message: change meaning of \"empty merge message\"","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2017-07-06T04:46:40Z","receivedAt":"2017-07-06T04:46:47Z","isPatch":true,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"On Thu, Jul 06, 2017 at 09:01:49AM +0530, Kaartic Sivaraam wrote:\n> In the context of \"git merge\" the meaning of an \"empty message\"\n> is one that contains no line of text. This is not in line with\n> \"git commit\" where an \"empty message\" is one that contains only\n> whitespaces and/or signed-off-by lines. This could cause surprises\n> to users who are accustomed to the meaning of an \"empty message\"\n> of \"git commit\".\n> \n> Prevent such surprises by changing the meaning of an empty 'merge\n> message' to be in line with that of an empty 'commit message'.\n> \n> Signed-off-by: Kaartic Sivaraam \n> ---\n> builtin/merge.c | 35 ++++++++++++++++++++++++++++++++++-\n> 1 file changed, 34 insertions(+), 1 deletion(-)\n> \n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 703827f00..db4bf1c40 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -748,6 +748,39 @@ static void abort_commit(struct commit_list *remoteheads, const char *err_msg)\n> \texit(1);\n> }\n> \n> +/*\n> + * Find out if the message in the strbuf contains only whitespace and\n> + * Signed-off-by lines.\n> + *\n> + * This function is the \"rest_is_space\" function of \"commit\" with the unwanted\n> + * parameter removed.\n\nThe function is called \"rest_is_empty\".\n\nBut isn't it better that commit and merge use the same code, instead of\nduplicating it again? Otherwise one may be updated, and the other\nforgotten, getting differences in behaviur, which is what you want to\nsolve.\n\nKevin\n\n"},{"id":"323890","messageId":"1499343609.2239.3.camel@gmail.com","threadId":"46304","inReplyTo":"20170706044640.GA11020@alpha.vpn.ikke.info","subject":"Re: [PATCH] merge-message: change meaning of \"empty merge message\"","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-07-06T12:20:09Z","receivedAt":"2017-07-06T12:20:24Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Thu, 2017-07-06 at 06:46 +0200, Kevin Daudt wrote:\n> \n> The function is called \"rest_is_empty\".\n> \nThanks for correcting that!\n\n> But isn't it better that commit and merge use the same code, instead\n> of\n> duplicating it again? Otherwise one may be updated, and the other\n> forgotten, getting differences in behaviur, which is what you want to\n> solve.\n> \nYes, I did think of that. It *seems* that neither \"message_is_empty\" or\nthe \"rest_is_empty\" are exposed to other files. Have to work on that.\n\n-- \nKaartic\n"},{"id":"324192","messageId":"20170711141254.7747-1-kaarticsivaraam91196@gmail.com","threadId":"46304","inReplyTo":"20170706044640.GA11020@alpha.vpn.ikke.info","subject":"[PATCH] commit & merge: modularize the empty message validator","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-07-11T14:12:54Z","receivedAt":"2017-07-11T14:12:58Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"In the context of \"git merge\" the meaning of an \"empty message\"\nis one that contains no line of text. This is not in line with\n\"git commit\" where an \"empty message\" is one that contains only\nwhitespaces and/or signed-off-by lines. This could cause surprises\nto users who are accustomed to the meaning of an \"empty message\"\nof \"git commit\".\n\nPrevent such surprises by ensuring the meaning of an empty 'merge\nmessage' to be in line with that of an empty 'commit message'. This\nis done by separating the empty message validator from 'commit' and\nmaking it stand-alone.\n\nSigned-off-by: Kaartic Sivaraam \n---\n I have made an attempt to solve the issue by separating the concerned\n function as I found no reason against it.\n\n I've tried to name them with what felt appropriate and concise to me.\n Let me know if it's alright.\n \n Makefile | 1 +\n builtin/commit.c | 39 +++++----------------------------------\n builtin/merge.c | 3 ++-\n message-validator.c | 34 ++++++++++++++++++++++++++++++++++\n message-validator.h | 6 ++++++\n 5 files changed, 48 insertions(+), 35 deletions(-)\n create mode 100644 message-validator.c\n create mode 100644 message-validator.h\n\ndiff --git a/Makefile b/Makefile\nindex ffa6da71b..c1c26e434 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -783,6 +783,7 @@ LIB_OBJS += merge.o\n LIB_OBJS += merge-blobs.o\n LIB_OBJS += merge-recursive.o\n LIB_OBJS += mergesort.o\n+LIB_OBJS += message-validator.o\n LIB_OBJS += mru.o\n LIB_OBJS += name-hash.o\n LIB_OBJS += notes.o\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8d1cac062..4c3112bb4 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -33,6 +33,7 @@\n #include \"notes-utils.h\"\n #include \"mailmap.h\"\n #include \"sigchain.h\"\n+#include \"message-validator.h\"\n \n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [] [--] ...\"),\n@@ -979,41 +980,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \treturn 1;\n }\n \n-static int rest_is_empty(struct strbuf *sb, int start)\n-{\n-\tint i, eol;\n-\tconst char *nl;\n-\n-\t/* Check if the rest is just whitespace and Signed-of-by's. */\n-\tfor (i = start; i < sb->len; i++) {\n-\t\tnl = memchr(sb->buf + i, '\\n', sb->len - i);\n-\t\tif (nl)\n-\t\t\teol = nl - sb->buf;\n-\t\telse\n-\t\t\teol = sb->len;\n-\n-\t\tif (strlen(sign_off_header) <= eol - i &&\n-\t\t starts_with(sb->buf + i, sign_off_header)) {\n-\t\t\ti = eol;\n-\t\t\tcontinue;\n-\t\t}\n-\t\twhile (i < eol)\n-\t\t\tif (!isspace(sb->buf[i++]))\n-\t\t\t\treturn 0;\n-\t}\n-\n-\treturn 1;\n-}\n-\n-/*\n- * Find out if the message in the strbuf contains only whitespace and\n- * Signed-off-by lines.\n- */\n-static int message_is_empty(struct strbuf *sb)\n+static int is_empty(struct strbuf *sb)\n {\n \tif (cleanup_mode == CLEANUP_NONE && sb->len)\n \t\treturn 0;\n-\treturn rest_is_empty(sb, 0);\n+\treturn message_is_empty(sb, 0);\n }\n \n /*\n@@ -1035,7 +1006,7 @@ static int template_untouched(struct strbuf *sb)\n \tif (!skip_prefix(sb->buf, tmpl.buf, &start))\n \t\tstart = sb->buf;\n \tstrbuf_release(&tmpl);\n-\treturn rest_is_empty(sb, start - sb->buf);\n+\treturn message_is_empty(sb, start - sb->buf);\n }\n \n static const char *find_author_by_nickname(const char *name)\n@@ -1744,7 +1715,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tfprintf(stderr, _(\"Aborting commit; you did not edit the message.\\n\"));\n \t\texit(1);\n \t}\n-\tif (message_is_empty(&sb) && !allow_empty_message) {\n+\tif (is_empty(&sb) && !allow_empty_message) {\n \t\trollback_index_files();\n \t\tfprintf(stderr, _(\"Aborting commit due to empty commit message.\\n\"));\n \t\texit(1);\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 703827f00..625cfb848 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -31,6 +31,7 @@\n #include \"gpg-interface.h\"\n #include \"sequencer.h\"\n #include \"string-list.h\"\n+#include \"message-validator.h\"\n \n #define DEFAULT_TWOHEAD (1<<0)\n #define DEFAULT_OCTOPUS (1<<1)\n@@ -772,7 +773,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t}\n \tread_merge_msg(&msg);\n \tstrbuf_stripspace(&msg, 0 < option_edit);\n-\tif (!msg.len)\n+\tif (!msg.len || message_is_empty(&msg, 0))\n \t\tabort_commit(remoteheads, _(\"Empty commit message.\"));\n \tstrbuf_release(&merge_msg);\n \tstrbuf_addbuf(&merge_msg, &msg);\ndiff --git a/message-validator.c b/message-validator.c\nnew file mode 100644\nindex 000000000..32feb4e26\n--- /dev/null\n+++ b/message-validator.c\n@@ -0,0 +1,34 @@\n+#include \"git-compat-util.h\"\n+#include \"sequencer.h\"\n+#include \"strbuf.h\"\n+#include \"message-validator.h\"\n+\n+/*\n+ * Find out if the message in the strbuf contains only whitespace and\n+ * Signed-off-by lines.\n+ */\n+int message_is_empty(struct strbuf *sb, int start)\n+{\n+\tint i, eol;\n+\tconst char *nl;\n+\n+\t/* Check if the rest is just whitespace and Signed-of-by's. */\n+\tfor (i = start; i < sb->len; i++) {\n+\t\tnl = memchr(sb->buf + i, '\\n', sb->len - i);\n+\t\tif (nl)\n+\t\t\teol = nl - sb->buf;\n+\t\telse\n+\t\t\teol = sb->len;\n+\n+\t\tif (strlen(sign_off_header) <= eol - i &&\n+\t\t starts_with(sb->buf + i, sign_off_header)) {\n+\t\t\ti = eol;\n+\t\t\tcontinue;\n+\t\t}\n+\t\twhile (i < eol)\n+\t\t\tif (!isspace(sb->buf[i++]))\n+\t\t\t\treturn 0;\n+\t}\n+\n+\treturn 1;\n+}\ndiff --git a/message-validator.h b/message-validator.h\nnew file mode 100644\nindex 000000000..4caea499c\n--- /dev/null\n+++ b/message-validator.h\n@@ -0,0 +1,6 @@\n+#ifndef MESSAGE_VALIDATOR_H\n+#define MESSAGE_VALIDATOR_H\n+\n+extern int message_is_empty(struct strbuf *sb, int start);\n+\n+#endif\n-- \n2.13.2.957.g457671ade\n\n"},{"id":"324194","messageId":"1499784112.9558.1.camel@gmail.com","threadId":"46304","inReplyTo":"20170711141254.7747-1-kaarticsivaraam91196@gmail.com","subject":"Re: [PATCH/RFC] commit & merge: modularize the empty message validator","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-07-11T14:41:52Z","receivedAt":"2017-07-11T14:41:53Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"Sorry, forgot to add the RFC suffix to [PATCH]. Please consider it's\npresence.\n\n-- \nKaartic\n"},{"id":"324230","messageId":"xmqq8tju3eqp.fsf@gitster.mtv.corp.google.com","threadId":"46304","inReplyTo":"20170711141254.7747-1-kaarticsivaraam91196@gmail.com","subject":"Re: [PATCH] commit & merge: modularize the empty message validator","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-11T20:22:54Z","receivedAt":"2017-07-11T20:23:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kaartic Sivaraam writes:\n\n> In the context of \"git merge\" the meaning of an \"empty message\"\n> is one that contains no line of text. This is not in line with\n> \"git commit\" where an \"empty message\" is one that contains only\n> whitespaces and/or signed-off-by lines. This could cause surprises\n> to users who are accustomed to the meaning of an \"empty message\"\n> of \"git commit\".\n>\n> Prevent such surprises by ensuring the meaning of an empty 'merge\n> message' to be in line with that of an empty 'commit message'. This\n> is done by separating the empty message validator from 'commit' and\n> making it stand-alone.\n>\n> Signed-off-by: Kaartic Sivaraam \n> ---\n> I have made an attempt to solve the issue by separating the concerned\n> function as I found no reason against it.\n>\n> I've tried to name them with what felt appropriate and concise to me.\n> Let me know if it's alright.\n\nI probably would have avoided a pair of new files just to house a\nsingle function. I anticipate that the last helper function in\ncommit.c at the top-level would become relevant to this topic, and\nbecause of that, I would have added this function at the end of the\nfile if I were doing this patch.\n\n> @@ -772,7 +773,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n> \t}\n> \tread_merge_msg(&msg);\n> \tstrbuf_stripspace(&msg, 0 < option_edit);\n> -\tif (!msg.len)\n> +\tif (!msg.len || message_is_empty(&msg, 0))\n\nI do not see much point in checking !msg.len here. The function\nimmediately returns by hitting the termination condition of the\noutermost loop and this is not a performance-critical codepath.\n\nI think the \"validation\" done with the rest_is_empty() is somewhat\nbogus. Why should we reject a commit without a message and a\ntrailer block with only signed-off-by lines, while accepting a\ncommit without a message and a trailer block as long as the trailer\nblock has something equally meaningless by itself, like\n\"Helped-by:\"? I think we should inspect the proposed commit log\nmessage taken from the editor, find its tail ignoring the trailing\ncomment using ignore_non_trailer, and further separate the result\ninto (, , ) using the same\nlogic used by the interpret-trailers tool, and then complain when\n turns out to be empty, to be truly useful and consistent.\n\nAnd for that eventual future, merging the logic used in commit and\nmerge might be a good first step.\n\nHaving said all that, I am not sure \"Prevent such surprises\" is a\nproblem that is realistic to begin with. When a user sees the\neditor buffer in \"git merge\", it is pre-populated with at least a\nsingle line of message \"Merge branch 'foo'\", possibly followed by\nthe summary of the side branch being merged, so unless the user\ndeliberately removes everything and then add a sign-off line\n(because we do not usually add one), there is no room for \"such\nsurprises\" in the first place. It does not _hurt_ to diagnose such\na crazy case, but it feels a bit lower priority.\n\nSo from the point of \"let's improve what merge does\", this change\nlooks to me a borderline \"Meh\"; but to improve the \"why sign-off is\nso special and behave differently from helped-by when deciding if\nthere is any log?\" situation, having a separate helper function that\nis shared across multiple codepaths that accept edited result may be\na good idea.\n\n"},{"id":"324349","messageId":"1499950837.2427.1.camel@gmail.com","threadId":"46304","inReplyTo":"xmqq8tju3eqp.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] commit & merge: modularize the empty message validator","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-07-13T13:00:37Z","receivedAt":"2017-07-13T13:00:32Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Tue, 2017-07-11 at 13:22 -0700, Junio C Hamano wrote:\n> Having said all that, I am not sure \"Prevent such surprises\" is a\n> problem that is realistic to begin with.  When a user sees the\n> editor buffer in \"git merge\", it is pre-populated with at least a\n> single line of message \"Merge branch 'foo'\", possibly followed by\n> the summary the side branch being merged, so unless the user\n> deliberately removes everything and then add a sign-off line\n> (because we do not usually add one), there is no room for \"such\n> surprises\" in the first place.  It does not _hurt_ to diagnose such\n> a crazy case, but it feels a bit lower priority.\n> \nIt's little unfortunate that I haven't mentioned the reason I asked the\nquestion that has resulted in this patch. It would explain a little\nabout why I thought this wasn't \"meh\" (I hope it stands for \"who care\nwhat ever\").\n\nSometimes I abort an commit from from the editor by providing an empty\ncommit message. Then I came to know that 'git commit' considers commit\nmessages with just signed-off-by lines as an empty message. I tried to\ntake advantage of that. I once tried to abort a merge by just removing\nthe \"Merge ...\" line and leaving the \"Signed-off\" line and was\nsurprised to see the merge happen instead of an abort. The rest is\nhistory. :)\n\n-- \nKaartic\n"},{"id":"324400","messageId":"xmqqr2xkxlpo.fsf@gitster.mtv.corp.google.com","threadId":"46304","inReplyTo":"1499950837.2427.1.camel@gmail.com","subject":"Re: [PATCH] commit & merge: modularize the empty message validator","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-13T17:58:43Z","receivedAt":"2017-07-13T17:58:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kaartic Sivaraam writes:\n\n> Sometimes I abort an commit from from the editor by providing an empty\n> commit message. Then I came to know that 'git commit' considers commit\n> messages with just signed-off-by lines as an empty message. I tried to\n> take advantage of that. I once tried to abort a merge by just removing\n> the \"Merge ...\" line and leaving the \"Signed-off\" line and was\n> surprised to see the merge happen instead of an abort. The rest is\n> history. :)\n\nI think many people know about and do use the \"delete all lines\"\n(i.e. \":1,$d\" in vi, or \\M-< \\C-SPC \\M-> \\C-w in Emacs) to abort out\nof a commit or a merge. I just do not think it is likely for them\nto leave Sign-off lines and remove everything else, which is more\nwork than to delete everything, hence my reaction.\n\n"},{"id":"324403","messageId":"1499969722.5973.2.camel@gmail.com","threadId":"46304","inReplyTo":"xmqq8tju3eqp.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] commit & merge: modularize the empty message validator","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-07-13T18:15:22Z","receivedAt":"2017-07-13T18:15:16Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Tue, 2017-07-11 at 13:22 -0700, Junio C Hamano wrote:\n> I think the \"validation\" done with the rest_is_empty() is somewhat\n> bogus.  Why should we reject a commit without a message and a\n> trailer block with only signed-off-by lines, while accepting a\n> commit without a message and a trailer block as long as the trailer\n> block has something equally meaningless by itself, like\n> \"Helped-by:\"?  I think we should inspect the proposed commit log\n> message taken from the editor, find its tail ignoring the trailing\n> comment using ignore_non_trailer, and further separate the result\n> into (, , ) using the same\n> logic used by the interpret-trailers tool, and then complain when\n> turns out to be empty, to be truly useful and consistent.\n> \nI have a few doubts for which I need clarification to move on with\nthis. \n\n 1. If we abort when the part is empty wouldn't it be too\n restrictive ?\n\n IOW, Wouldn't it affect users of \"git commit -‍-cleanup=verbatim\"\n who wish to commit only the comments or parts of it ?\n (I'm not sure if someone would find that useful)\n\n 2. Is it ok to use the \"find_trailer_start\" function of \"trailer.c\"\n to locate the trailer? \n\n Note: It has a little issue that it wouldn't detect the trailer if\n the message comprises of one trailer alone and no other text. This\n case occurs while aborting a commit started using \"git commit -s\".\n Any possibilities to overcome the issue?\n\n 3. Ignoring point 1 for now, What other helper methods except the\n ones listed below could be helpful in the separating the cleaned up\n commit message into the , , ?\n\n * ignore_non_trailer\n * find_trailer_start\n\n-- \nKaartic\n"},{"id":"324419","messageId":"xmqqiniww37i.fsf@gitster.mtv.corp.google.com","threadId":"46304","inReplyTo":"1499969722.5973.2.camel@gmail.com","subject":"Re: [PATCH] commit & merge: modularize the empty message validator","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-13T19:23:45Z","receivedAt":"2017-07-13T19:23:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kaartic Sivaraam writes:\n\n> I have a few doubts for which I need clarification to move on with\n> this.\n>\n> 1. If we abort when the part is empty wouldn't it be too\n> restrictive ?\n>\n> IOW, Wouldn't it affect users of \"git commit -‍-cleanup=verbatim\"\n> who wish to commit only the comments or parts of it ?\n> (I'm not sure if someone would find that useful)\n>\n> 2. Is it ok to use the \"find_trailer_start\" function of \"trailer.c\"\n> to locate the trailer? \n>\n> Note: It has a little issue that it wouldn't detect the trailer if\n> the message comprises of one trailer alone and no other text. This\n> case occurs while aborting a commit started using \"git commit -s\".\n> Any possibilities to overcome the issue?\n>\n> 3. Ignoring point 1 for now, What other helper methods except the\n> ones listed below could be helpful in the separating the cleaned up\n> commit message into the , , ?\n>\n> * ignore_non_trailer\n> * find_trailer_start\n\nAll good points; if it bothers you that \"commit\" and \"merge\" define\n\"emptyness\" of the buffer differently too much, I think you could\npersuade me to unify them to \"the buffer _must_ contain no bytes\",\ni.e. not special-casing sign-off lines only \"commit\".\n\nIt would be a backward incompatible tightening of the established\nrule, but it may not be a bad change.\n"},{"id":"324505","messageId":"1500039101.1939.3.camel@gmail.com","threadId":"46304","inReplyTo":"xmqqr2xkxlpo.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] commit & merge: modularize the empty message validator","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-07-14T13:31:41Z","receivedAt":"2017-07-14T13:31:32Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Thu, 2017-07-13 at 10:58 -0700, Junio C Hamano wrote:\n> I think many people know about and do use the \"delete all lines\"\n> (i.e. \":1,$d\" in vi, or \\M-< \\C-SPC \\M-> \\C-w in Emacs) to abort out\n> of a commit or a merge.  I just do not think it is likely for them\n> to leave Sign-off lines and remove everything else, which is more\n> work than to delete everything, hence my reaction.\n> \nThanks! Didn't know this before.\n\n"},{"id":"324542","messageId":"1500054552.18990.8.camel@gmail.com","threadId":"46304","inReplyTo":"xmqqiniww37i.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] commit & merge: modularize the empty message validator","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-07-14T17:49:12Z","receivedAt":"2017-07-14T17:54:20Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Thu, 2017-07-13 at 12:23 -0700, Junio C Hamano wrote:\n> All good points; if it bothers you that \"commit\" and \"merge\" define\n> \"emptyness\" of the buffer differently too much, I think you could\n> persuade me to unify them to \"the buffer _must_ contain no bytes\",\n> i.e. not special-casing sign-off lines only \"commit\".\n> \nIntereseting, let me give it a try.\n\nTo persuade you with this, I have to convince you that the current\nbehaviour (special-casing of sign-off lines) is defective and/or\nbiased. It really is for quite a few reasons,\n\n * Though it's not apparent, it indirectly seems to be hindering\n (to some extent) the idea of including the sign-off (or) other\n trailers which *can't be modified* by the user.\n\n IOW, the current behaviour seems make the contributors/users\n falsely believe (at least to some extent) that git *does* have\n trailers for commit messages and thus preventing them from coming\n up with ideas that could make \"untouchable trailers\" a reality.\n\n Thus, consider \"the buffer _must_ contain no bytes\" hoping this\n would initiate a \"Butterfly effect\" :)\n\n\n * Looking from an implementation perspective, it's biased in that\n it checks only for sign-offs. Making it work in general is\n difficult as there's no standard definition for the term\n . That's because it varies with respect to usage, I\n think. Different people/projects may consider different lines to\n be trailer lines. A few examples are,\n\n * Bug:\n * Fixes:\n * Change-id:\n * Helped-by:\n\n Moreover, some people may wish to have commit messages that only\n have such trailers (e.g. \"Fixes:\"). So, it's difficult to do a\n generalized implementation that aborts when the message is empty\n or consists only of trailers.\n\n Thus, consider \"the buffer _must_ contain no bytes\" because it's\n not easy to define what a means and special casing\n \"sign-off\" is biased.\n\n\n * Imagine a hypothetical version of git that aborts when the\n is empty though a  is present. This would\n quite possibly instigate controversies as the \"hypothetical git\"\n reduces the \"valid commit messages\" and would quite possibly\n reject a commit message as \"empty\" (which is uncommunicative)\n though a previous version (which did not have this change)\n accepted a similar message.\n\n SO, bringing in the Occam's razor, let's choose the option that's\n the simplest and makes the fewest assumptions.\n\n\nThus, I conclude that the considering a commit message consisting only\nof s as empty isn't a good idea and should be dropped.\n\n\n-- \nKaartic\n"},{"id":"324568","messageId":"1500107583.1850.4.camel@gmail.com","threadId":"46304","inReplyTo":"1500054552.18990.8.camel@gmail.com","subject":"Re: [PATCH] commit & merge: modularize the empty message validator","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-07-15T08:33:03Z","receivedAt":"2017-07-15T08:32:56Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Fri, 2017-07-14 at 23:19 +0530, Kaartic Sivaraam wrote:\n> * Imagine a hypothetical version of git that aborts when the\n> is empty though a  is present. This would\n> quite possibly instigate controversies as the \"hypothetical git\"\n> reduces the \"valid commit messages\" and would quite possibly\n> reject a commit message as \"empty\" (which is uncommunicative)\n> though a previous version (which did not have this change)\n> accepted a similar message.\n> \n> SO, bringing in the Occam's razor, let's choose the option that's\n> the simplest and makes the fewest assumptions.\nI would like to add a little to the \"making fewer assumptions\" point.\nIf we make the fewest assumptions possible, it has quite a few\nadvantages,\n\n* It would make the implementation that checks for an empty message,\ntrivial. Thus reducing the complexity of the code.\n\n* It would not overload the meaning of the error message,\n\n Aborting due to empty commit message.\n\nThus making the sentence stand for what it means \"literally\". \n(BTW, I guess an \"an\" is missing in the message)\n\n* It allows for others to have more freedom in defining what a commit\nmessage should have using the appropriate hook(s). IOW, let us do the\nminimal check(message consisting only of whitespaces) and let the\nothers define what a commit message should have using the \"commit-msg\"\nhook.\n\n-- \nKaartic\n"},{"id":"324639","messageId":"20170717090838.GA17826@256bit.org","threadId":"46304","inReplyTo":"xmqqr2xkxlpo.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] commit & merge: modularize the empty message validator","fromName":"Christian Brabandt","fromEmail":"cb@256bit.org","sentAt":"2017-07-17T09:08:38Z","receivedAt":"2017-07-17T09:52:16Z","isPatch":true,"sender":{"key":"cb@256bit.org","avatar":"https://gravatar.com/avatar/c72756321dd9fa10cada6d4b1f2c4e577a979373d76960608f816cb49c575b02?d=mp&s=160"},"body":"\nOn Do, 13 Jul 2017, Junio C Hamano wrote:\n\n> I think many people know about and do use the \"delete all lines\"\n> (i.e. \":1,$d\" in vi, or \\M-< \\C-SPC \\M-> \\C-w in Emacs) to abort out\n> of a commit or a merge. I just do not think it is likely for them\n> to leave Sign-off lines and remove everything else, which is more\n> work than to delete everything, hence my reaction.\n\nIn Vim you can also abort the commit message using :cq which exits the \neditor with an error code.\n\nBest,\nChristian\n-- \nDas Werk soll den Meister loben.\n"},{"id":"324649","messageId":"xmqq379vhtkk.fsf@gitster.mtv.corp.google.com","threadId":"46304","inReplyTo":"20170717090838.GA17826@256bit.org","subject":"Re: [PATCH] commit & merge: modularize the empty message validator","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-17T17:16:59Z","receivedAt":"2017-07-17T17:17:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Brabandt writes:\n\n> On Do, 13 Jul 2017, Junio C Hamano wrote:\n>\n>> I think many people know about and do use the \"delete all lines\"\n>> (i.e. \":1,$d\" in vi, or \\M-< \\C-SPC \\M-> \\C-w in Emacs) to abort out\n>> of a commit or a merge. I just do not think it is likely for them\n>> to leave Sign-off lines and remove everything else, which is more\n>> work than to delete everything, hence my reaction.\n>\n> In Vim you can also abort the commit message using :cq which exits the \n> editor with an error code.\n\nSure, but it's not like we are trying to come up with an education\nmaterial to teach people how to abort their commit in progress in\nthis discussion, so I do not quite see a relevance of your comment\nto the topic at hand here.\n"},{"id":"326875","messageId":"20170821133440.5552-1-kaarticsivaraam91196@gmail.com","threadId":"46304","inReplyTo":"1500107583.1850.4.camel@gmail.com","subject":"[PATCH v2] branch: change the error messages to be more meaningful","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-08-21T13:34:40Z","receivedAt":"2017-08-21T13:38:20Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"The error messages shown when the branch command is misused\nby supplying it wrong number of parameters wasn't meaningful.\nThat's because it used the the phrase \"too many branches\"\nassuming all parameters to be \"valid\" branch names. It's not\nalways the case as exemplified below,\n\n $ git branch\n foo\n * master\n\n $ git branch -m foo foo old\n fatal: too many branches for a rename operation\n\nChange the messages to be more general thus making no assumptions\nabout the \"parameters\".\n\nSigned-off-by: Kaartic Sivaraam \n---\n Changes in v2:\n\n - changed the wordings of the error message\n\n builtin/branch.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex a3bd2262b..62981d358 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -707,12 +707,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\telse if (argc == 2)\n \t\t\trename_branch(argv[0], argv[1], rename > 1);\n \t\telse\n-\t\t\tdie(_(\"too many branches for a rename operation\"));\n+\t\t\tdie(_(\"too many arguments for a rename operation\"));\n \t} else if (new_upstream) {\n \t\tstruct branch *branch = branch_get(argv[0]);\n \n \t\tif (argc > 1)\n-\t\t\tdie(_(\"too many branches to set new upstream\"));\n+\t\t\tdie(_(\"too many arguments to set new upstream\"));\n \n \t\tif (!branch) {\n \t\t\tif (!argc || !strcmp(argv[0], \"HEAD\"))\n@@ -735,7 +735,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tstruct strbuf buf = STRBUF_INIT;\n \n \t\tif (argc > 1)\n-\t\t\tdie(_(\"too many branches to unset upstream\"));\n+\t\t\tdie(_(\"too many arguments to unset upstream\"));\n \n \t\tif (!branch) {\n \t\t\tif (!argc || !strcmp(argv[0], \"HEAD\"))\n-- \n2.14.0.rc1.434.g6eded367a\n\n"},{"id":"326876","messageId":"1503323533.2210.7.camel@gmail.com","threadId":"46304","inReplyTo":"20170821133440.5552-1-kaarticsivaraam91196@gmail.com","subject":"Re: [PATCH v2] branch: change the error messages to be more meaningful","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-08-21T13:52:13Z","receivedAt":"2017-08-21T13:51:24Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"Sorry, wrong thread :( Please ignore this.\n\n---\nKaartic\n"},{"id":"326877","messageId":"20170821140528.7212-1-kaarticsivaraam91196@gmail.com","threadId":"46304","inReplyTo":"1500107583.1850.4.camel@gmail.com","subject":"[PATCH v2/RFC] commit: change the meaning of an empty commit message","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-08-21T14:05:28Z","receivedAt":"2017-08-21T14:04:55Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"An \"empty commit message\" according to 'commit' has long been,\n\n A message that contains only empty lines and/or whitespaces\n and/or 'Signed-off-by' lines\n\nThis is biased as a commit message that contains only other trailers like\n'Helped-by: ', 'Tested-by: ' etc., could equally be considered empty but\nsuch messages are considered valid. Detecting *all* possible trailers\nand aborting when a commit message contains only those trailers is not\nan easy thing as the meaning of a 'trailer' is not universal.\n\nFurther, leaving the meaning unchanged has the issue that it isn't\nconsistent with the meaning of an empty \"merge\" message which is,\n\n A message that contains only empty lines and/or whitespaces\n\nIn order to keep the implementation simple and to be consistent with\nthe meaning of an \"empty merge message\"and to remain unbiased redefine\nthe meaning of an \"empty commit message\" as,\n\n A message that contains only empty lines and/or whitespaces\n\nUsers who would like to have a different notion of an \"empty commit message\"\ncan do so using the 'commit-msg' hook.\n\nAs a result of this change, the following commit message which was rejected\nas empty before this change is considered to be valid as a consequence\nof this change.\n\n ---- START : COMMIT MESSAGE ----\n\n Signed-off-by: Random J Developer \n\n # Please enter the commit message for your changes. Lines starting\n # with '#' will be ignored, and an empty message aborts the commit.\n # ...\n ---- END : COMMIT MESSAGE ----\n\nWith the default cleanup, the above message would produce a commit with the\n'Signed-off-by:' line as it's subject. Eg,\n\n [master 4a34e74] Signed-off-by: Random J Developer \n\nSigned-off-by: Kaartic Sivaraam \n---\n\n As has been noted by Junio, \n\n \"It would be a backward incompatible tightening of the established\n rule, but it may not be a bad change.\"\n\n The \"It\" above refers to this change. Expecting comments from people to ensure\n this change isn't a bad one.\n\n Changes in v2:\n\n Unlike the previous patch this one \"doesn't add much\". Only the meaning of\n the empty commit message has been changed.\n\n Unlike the previous patch, this one doesn't touch on 'merge' because after\n this patch has been applied both commit and merge seem to reject the same set\n of messages as an empty message.\n\n I couldn't find the meaning of an empty commit message in any part of the\n documentation. Let me know if there's some doc to update.\n\n builtin/commit.c | 10 ++--------\n 1 file changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8e9380251..26636aac1 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -981,7 +981,7 @@ static int rest_is_empty(struct strbuf *sb, int start)\n \tint i, eol;\n \tconst char *nl;\n \n-\t/* Check if the rest is just whitespace and Signed-off-by's. */\n+\t/* Check if the rest is just whitespace */\n \tfor (i = start; i < sb->len; i++) {\n \t\tnl = memchr(sb->buf + i, '\\n', sb->len - i);\n \t\tif (nl)\n@@ -989,11 +989,6 @@ static int rest_is_empty(struct strbuf *sb, int start)\n \t\telse\n \t\t\teol = sb->len;\n \n-\t\tif (strlen(sign_off_header) <= eol - i &&\n-\t\t starts_with(sb->buf + i, sign_off_header)) {\n-\t\t\ti = eol;\n-\t\t\tcontinue;\n-\t\t}\n \t\twhile (i < eol)\n \t\t\tif (!isspace(sb->buf[i++]))\n \t\t\t\treturn 0;\n@@ -1003,8 +998,7 @@ static int rest_is_empty(struct strbuf *sb, int start)\n }\n \n /*\n- * Find out if the message in the strbuf contains only whitespace and\n- * Signed-off-by lines.\n+ * Find out if the message in the strbuf contains only whitespace\n */\n static int message_is_empty(struct strbuf *sb)\n {\n-- \n2.14.1.656.g66e7d6d0f\n\n"},{"id":"327157","messageId":"xmqqo9r4vhv0.fsf@gitster.mtv.corp.google.com","threadId":"46304","inReplyTo":"20170821140528.7212-1-kaarticsivaraam91196@gmail.com","subject":"Re: [PATCH v2/RFC] commit: change the meaning of an empty commit message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-24T20:19:31Z","receivedAt":"2017-08-24T20:19:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kaartic Sivaraam writes:\n\n> As has been noted by Junio, \n>\n> \"It would be a backward incompatible tightening of the established\n> rule, but it may not be a bad change.\"\n>\n> The \"It\" above refers to this change. Expecting comments from people to ensure\n> this change isn't a bad one.\n\nFWIW, I am fairly neutral; I do not mind accepting this change if\nother people are supportive, but I do not miss this patch if we end\nup not applying it at all. The latter is easier for me as we do not\nhave to worry about breaking people's scripts and tools used in\ntheir established workflows at all.\n\n\n"},{"id":"327482","messageId":"1504186577.1826.9.camel@gmail.com","threadId":"46304","inReplyTo":"xmqqo9r4vhv0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2/RFC] commit: change the meaning of an empty commit message","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-08-31T13:36:17Z","receivedAt":"2017-08-31T13:35:29Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Thu, 2017-08-24 at 13:19 -0700, Junio C Hamano wrote:\n> \n> The latter is easier for me as we do not have to worry about \n> breaking people's scripts and tools used in\n> their established workflows at all.\n> \n\nIn that case, how about doing something similar to what was done to\n'set-upstream' option of branch? We could print a warning notice when\nthe commit message is found to be empty due to the presence of a sign-\noff line. As usual we could stop warning and stop identifying log\nmessages consisting only signed-off lines as empty after a few years of\ndoing that.\n\nNote: I have no idea how good an idea this is. Let me know if it's a\nbad one.\n\n-- \nKaartic\n"},{"id":"329467","messageId":"1506964828.3504.5.camel@gmail.com","threadId":"46304","inReplyTo":"1504186577.1826.9.camel@gmail.com","subject":"Re: [PATCH v2/RFC] commit: change the meaning of an empty commit message","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-10-02T17:20:28Z","receivedAt":"2017-10-02T17:20:39Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Thu, 2017-08-31 at 19:06 +0530, Kaartic Sivaraam wrote:\n> On Thu, 2017-08-24 at 13:19 -0700, Junio C Hamano wrote:\n> > \n> > The latter is easier for me as we do not have to worry about \n> > breaking people's scripts and tools used in\n> > their established workflows at all.\n> > \n> \n> In that case, how about doing something similar to what was done to\n> 'set-upstream' option of branch? We could print a warning notice when\n> the commit message is found to be empty due to the presence of a sign-\n> off line. As usual we could stop warning and stop identifying log\n> messages consisting only signed-off lines as empty after a few years of\n> doing that.\n> \n> Note: I have no idea how good an idea this is. Let me know if it's a\n> bad one.\n> \n\n\nI was recently searching to find the patches have gone missing in to\nthe void for no obvious reason and found this. Should I consider this\nto be \"Dropped\" in terms of the \"What's cooking\" emails or has this\njust not received the required attention?\n\n---\nKaartic\n"}]}