{"thread":{"id":"51559","subject":"[PATCHv3] send-email: Ask if a patch should be sent twice","startedAt":"2019-07-30T20:33:33Z","lastAt":"2019-07-31T01:06:28Z","messageCount":7,"participants":["Dmitry Safonov","SZEDER Gábor","Junio C Hamano","Andrey Vagin","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"379609","messageId":"20190730203327.30958-1-dima@arista.com","threadId":"51559","inReplyTo":null,"subject":"[PATCHv3] send-email: Ask if a patch should be sent twice","fromName":"Dmitry Safonov","fromEmail":"dima@arista.com","sentAt":"2019-07-30T20:33:27Z","receivedAt":"2019-07-30T20:33:33Z","isPatch":false,"sender":{"key":"dima@arista.com","avatar":null},"body":"I was almost certain that git won't let me send the same patch twice,\nbut today I've managed to double-send a directory by a mistake:\n\tgit send-email --to linux-kernel@vger.kernel.org /tmp/timens/\n\t --cc 'Dmitry Safonov <0x7f454c46@gmail.com>' /tmp/timens/`\n\n[I haven't noticed that I put the directory twice ^^]\n\nPrevent this shipwreck from happening again by asking if a patch\nis sent multiple times on purpose.\n\nlink: https://lkml.kernel.org/r/4d53ebc7-d5b2-346e-c383-606401d19d3a@gmail.com\nCc: Andrei Vagin \nSuggested-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Dmitry Safonov \n---\nv3: Use `test_i18ngrep` instead of plain `grep`\nv2: Moved the check under --validate,\n fixed tests,\n added a new test,\n updated documentation for --validate\n\n Documentation/git-send-email.txt | 2 ++\n git-send-email.perl | 14 ++++++++++++++\n t/t9001-send-email.sh | 15 +++++++++++++++\n 3 files changed, 31 insertions(+)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex d93e5d0f58f0..0441bb1b5d3b 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -421,6 +421,8 @@ have been specified, in which case default to 'compose'.\n \t\t\t('auto', 'base64', or 'quoted-printable') is used;\n \t\t\tthis is due to SMTP limits as described by\n \t\t\thttp://www.ietf.org/rfc/rfc5322.txt.\n+\t\t*\tAsk confirmation before sending patches multiple times\n+\t\t\tif the supplied patches set overlaps.\n --\n +\n Default is the value of `sendemail.validate`; if this is not set,\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 5f92c89c1c1b..c1638d06f81d 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -688,6 +688,9 @@ sub is_format_patch_arg {\n @files = handle_backup_files(@files);\n \n if ($validate) {\n+\tmy %seen;\n+\tmy @dupes = grep { $seen{$_}++ } @files;\n+\n \tforeach my $f (@files) {\n \t\tunless (-p $f) {\n \t\t\tmy $error = validate_patch($f, $target_xfer_encoding);\n@@ -695,6 +698,17 @@ sub is_format_patch_arg {\n \t\t\t\t\t\t $f, $error);\n \t\t}\n \t}\n+\tif (@dupes) {\n+\t\tprintf(__(\"Patches specified several times: \\n\"));\n+\t\tprintf(__(\"%s \\n\" x @dupes), @dupes);\n+\t\t$_ = ask(__(\"Do you want to send those patches several times? Y/n \"),\n+\t\t\tdefault => \"y\",\n+\t\t\tvalid_re => qr/^(?:yes|y|no|n)/i);\n+\t\tif (/^n/i) {\n+\t\t\tcleanup_compose_files();\n+\t\t\texit(0);\n+\t\t}\n+\t}\n }\n \n if (@files) {\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 997f90b42b3e..496005af1763 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -555,6 +555,7 @@ test_expect_success $PREREQ 'In-Reply-To without --chain-reply-to' '\n \t\t--no-chain-reply-to \\\n \t\t--in-reply-to=\"$(cat expect)\" \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t--no-validate \\\n \t\t$patches $patches $patches \\\n \t\t2>errors &&\n \t# The first message is a reply to --in-reply-to\n@@ -577,6 +578,7 @@ test_expect_success $PREREQ 'In-Reply-To with --chain-reply-to' '\n \t\t--chain-reply-to \\\n \t\t--in-reply-to=\"$(cat expect)\" \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t--no-validate \\\n \t\t$patches $patches $patches \\\n \t\t2>errors &&\n \tsed -n -e \"s/^In-Reply-To: *\\(.*\\)/\\1/p\" msgtxt1 >actual &&\n@@ -589,6 +591,19 @@ test_expect_success $PREREQ 'In-Reply-To with --chain-reply-to' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success $PREREQ 'ask confirmation for double-send' '\n+\tclean_fake_sendmail &&\n+\techo y | \\\n+\t\tGIT_SEND_EMAIL_NOTTY=1 \\\n+\t\tgit send-email --from=author@example.com \\\n+\t\t\t--to=nobody@example.com \\\n+\t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t\t--validate \\\n+\t\t\t$patches $patches $patches \\\n+\t\t\t>stdout &&\n+\t! test_i18ngrep \"Patches specified several times: \" stdout\n+'\n+\n test_expect_success $PREREQ 'setup fake editor' '\n \twrite_script fake-editor <<-\\EOF\n \techo fake edit >>\"$1\"\n-- \n2.22.0\n\n"},{"id":"379614","messageId":"20190730211012.GS20404@szeder.dev","threadId":"51559","inReplyTo":"20190730203327.30958-1-dima@arista.com","subject":"Re: [PATCHv3] send-email: Ask if a patch should be sent twice","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-07-30T21:10:12Z","receivedAt":"2019-07-30T21:10:18Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Jul 30, 2019 at 09:33:27PM +0100, Dmitry Safonov wrote:\n> @@ -589,6 +591,19 @@ test_expect_success $PREREQ 'In-Reply-To with --chain-reply-to' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success $PREREQ 'ask confirmation for double-send' '\n> +\tclean_fake_sendmail &&\n> +\techo y | \\\n> +\t\tGIT_SEND_EMAIL_NOTTY=1 \\\n> +\t\tgit send-email --from=author@example.com \\\n> +\t\t\t--to=nobody@example.com \\\n> +\t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t\t\t--validate \\\n> +\t\t\t$patches $patches $patches \\\n> +\t\t\t>stdout &&\n> +\t! test_i18ngrep \"Patches specified several times: \" stdout\n\nYou should write this as 'test_i18ngrep ! <...>'. When running the\ntest with GIT_TEST_GETTEXT_POISON=true, then 'test_i18ngrep' is\nbasically a noop and always returns with success, the leading ! would\nturn that into a failure, which then would fail the test.\n\nSorry for not being specific enough.\n\n"},{"id":"379615","messageId":"e12b1d56-717e-2b1e-8475-9353a855e343@arista.com","threadId":"51559","inReplyTo":"20190730211012.GS20404@szeder.dev","subject":"Re: [PATCHv3] send-email: Ask if a patch should be sent twice","fromName":"Dmitry Safonov","fromEmail":"dima@arista.com","sentAt":"2019-07-30T21:19:08Z","receivedAt":"2019-07-30T21:19:13Z","isPatch":false,"sender":{"key":"dima@arista.com","avatar":null},"body":"On 7/30/19 10:10 PM, SZEDER Gábor wrote:\n> On Tue, Jul 30, 2019 at 09:33:27PM +0100, Dmitry Safonov wrote:\n>> @@ -589,6 +591,19 @@ test_expect_success $PREREQ 'In-Reply-To with --chain-reply-to' '\n>> \ttest_cmp expect actual\n>> '\n>> \n>> +test_expect_success $PREREQ 'ask confirmation for double-send' '\n>> +\tclean_fake_sendmail &&\n>> +\techo y | \\\n>> +\t\tGIT_SEND_EMAIL_NOTTY=1 \\\n>> +\t\tgit send-email --from=author@example.com \\\n>> +\t\t\t--to=nobody@example.com \\\n>> +\t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n>> +\t\t\t--validate \\\n>> +\t\t\t$patches $patches $patches \\\n>> +\t\t\t>stdout &&\n>> +\t! test_i18ngrep \"Patches specified several times: \" stdout\n> \n> You should write this as 'test_i18ngrep ! <...>'. When running the\n> test with GIT_TEST_GETTEXT_POISON=true, then 'test_i18ngrep' is\n> basically a noop and always returns with success, the leading ! would\n> turn that into a failure, which then would fail the test.\n> \n> Sorry for not being specific enough.\n\nNo worries, thanks for the review anyway, quite educative - I haven't\nworked much on i18-related things in other projects, so sorry for not\ngetting it straight away.\nWill resend shortly..\n\nThanks,\n Dmtiry\n"},{"id":"379624","messageId":"xmqq5znjrx0h.fsf@gitster-ct.c.googlers.com","threadId":"51559","inReplyTo":"20190730203327.30958-1-dima@arista.com","subject":"Re: [PATCHv3] send-email: Ask if a patch should be sent twice","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-30T22:13:50Z","receivedAt":"2019-07-30T22:13:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dmitry Safonov writes:\n\n> I was almost certain that git won't let me send the same patch twice,\n\nWhy? And more importantly, does it matter to readers of this\nmessage what you thought?\n\n> but today I've managed to double-send a directory by a mistake:\n> \tgit send-email --to linux-kernel@vger.kernel.org /tmp/timens/\n> \t --cc 'Dmitry Safonov <0x7f454c46@gmail.com>' /tmp/timens/`\n>\n> [I haven't noticed that I put the directory twice ^^]\n>\n> Prevent this shipwreck from happening again by asking if a patch\n> is sent multiple times on purpose.\n>\n> link: https://lkml.kernel.org/r/4d53ebc7-d5b2-346e-c383-606401d19d3a@gmail.com\n\nWhat does \"link:\" mean?\n\n> Cc: Andrei Vagin \n\nWhat's the significance for this project to record that this patch\nwas CCed to Andrei?\n\n> Suggested-by: Ævar Arnfjörð Bjarmason \n\nI think \"Helped-by:\" is a lot more appropriate, viewing the exchange\non v2 from the sideline.\n\n> Signed-off-by: Dmitry Safonov \n> ---\n\n> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n> index d93e5d0f58f0..0441bb1b5d3b 100644\n> --- a/Documentation/git-send-email.txt\n> +++ b/Documentation/git-send-email.txt\n> @@ -421,6 +421,8 @@ have been specified, in which case default to 'compose'.\n> \t\t\t('auto', 'base64', or 'quoted-printable') is used;\n> \t\t\tthis is due to SMTP limits as described by\n> \t\t\thttp://www.ietf.org/rfc/rfc5322.txt.\n> +\t\t*\tAsk confirmation before sending patches multiple times\n> +\t\t\tif the supplied patches set overlaps.\n> --\n> +\n> Default is the value of `sendemail.validate`; if this is not set,\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 5f92c89c1c1b..c1638d06f81d 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -688,6 +688,9 @@ sub is_format_patch_arg {\n> @files = handle_backup_files(@files);\n> \n> if ($validate) {\n> +\tmy %seen;\n> +\tmy @dupes = grep { $seen{$_}++ } @files;\n> +\n> \tforeach my $f (@files) {\n> \t\tunless (-p $f) {\n> \t\t\tmy $error = validate_patch($f, $target_xfer_encoding);\n> @@ -695,6 +698,17 @@ sub is_format_patch_arg {\n> \t\t\t\t\t\t $f, $error);\n> \t\t}\n\nThis is not a fault of your patch at all, but the two hunks are\ncurious. If \"git format-patch\" chose to coalesce these two hunks\ninto one, the second hunk header can be replaced by\n\n\t\t$error and die sprintf(__(\"fatal: ...\"),\n\nThe end result would be that we will spend the same number of lines\nand we will see more useful information.\n\n> \t}\n> +\tif (@dupes) {\n> +\t\tprintf(__(\"Patches specified several times: \\n\"));\n> +\t\tprintf(__(\"%s \\n\" x @dupes), @dupes);\n> +\t\t$_ = ask(__(\"Do you want to send those patches several times? Y/n \"),\n> +\t\t\tdefault => \"y\",\n> +\t\t\tvalid_re => qr/^(?:yes|y|no|n)/i);\n> +\t\tif (/^n/i) {\n> +\t\t\tcleanup_compose_files();\n> +\t\t\texit(0);\n> +\t\t}\n> +\t}\n\nPerhaps this should be inserted _before_ the \"let's examine each\npatchfile and complain\" loop. Otherwise, you'd see this warning\nonly after seeing the same \"the file has too long a line\" error\non the same patch.\n\nWhile you are counting with %seen how many times the contents of\n@files appear, perhaps you can also create a list of unique files,\nso that you do not have to call validate_patch() more than once\nfor each of them. It would also allow you to offer another choice\nin the above question \"do you want to send them more than once?\",\nwhich may be much more useful than yes/no: \"drop duplicates\". If\nyou did so, you do not need to swap the order of the checks. You\nwould first count the occurences of each element in @files, then\ncall validate_patch() on each of them just once, and after you are\ndone, check if the user wants to send duplicates, abort, or dedup.\n\nPerhaps like this:\n\n if ($validate) {\n+\tmy (@dupes, %seen, @uniq);\n+\n\tforeach my $f (@files) {\n+\t\tif ($seen{$f}) {\n+\t\t\tif ($seen{$f} == 1) { push @dupes, $f; }\n+\t\t\tnext;\n+\t\t}\n+\t\t$seen{$f}++;\n+\t\tpush @uniq, $f;\n+\t}\n+\tforeach my $f (@uniq) {\t\n\t\tunless (-p $f) {\n\t\t\tmy $error = validate_patch(...);\n\t\t... the existing loop ...\n\t}\n+\n+\tif (@dupes) {\n+\t\t... the new code in your patch ...\n\n"},{"id":"379625","messageId":"CANaxB-yygbxt=-sP+cyhR9WhcyEck+Wy=453huwfVNC9+So0BA@mail.gmail.com","threadId":"51559","inReplyTo":"xmqq5znjrx0h.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCHv3] send-email: Ask if a patch should be sent twice","fromName":"Andrey Vagin","fromEmail":"avagin@openvz.org","sentAt":"2019-07-30T22:56:43Z","receivedAt":"2019-07-30T22:56:58Z","isPatch":false,"sender":{"key":"avagin@openvz.org","avatar":null},"body":"On Tue, Jul 30, 2019 at 3:13 PM Junio C Hamano wrote:\n>\n> Dmitry Safonov writes:\n>\n> > I was almost certain that git won't let me send the same patch twice,\n>\n> Why? And more importantly, does it matter to readers of this\n> message what you thought?\n\nSounds rude. What matter to readers except author's thoughts? I guess you want\nto say that the comment should be in more neutral technical form without\npersonal pronouns.\n\n>\n> > but today I've managed to double-send a directory by a mistake:\n> > git send-email --to linux-kernel@vger.kernel.org /tmp/timens/\n> > --cc 'Dmitry Safonov <0x7f454c46@gmail.com>' /tmp/timens/`\n> >\n> > [I haven't noticed that I put the directory twice ^^]\n> >\n> > Prevent this shipwreck from happening again by asking if a patch\n> > is sent multiple times on purpose.\n> >\n> > link: https://lkml.kernel.org/r/4d53ebc7-d5b2-346e-c383-606401d19d3a@gmail.com\n>\n> What does \"link:\" mean?\n>\n> > Cc: Andrei Vagin \n>\n> What's the significance for this project to record that this patch\n> was CCed to Andrei?\n>\n> > Suggested-by: Ævar Arnfjörð Bjarmason \n>\n> I think \"Helped-by:\" is a lot more appropriate, viewing the exchange\n> on v2 from the sideline.\n>\n> > Signed-off-by: Dmitry Safonov \n> > ---\n>\n> > diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n> > index d93e5d0f58f0..0441bb1b5d3b 100644\n> > --- a/Documentation/git-send-email.txt\n> > +++ b/Documentation/git-send-email.txt\n> > @@ -421,6 +421,8 @@ have been specified, in which case default to 'compose'.\n> > ('auto', 'base64', or 'quoted-printable') is used;\n> > this is due to SMTP limits as described by\n> > http://www.ietf.org/rfc/rfc5322.txt.\n> > + * Ask confirmation before sending patches multiple times\n> > + if the supplied patches set overlaps.\n> > --\n> > +\n> > Default is the value of `sendemail.validate`; if this is not set,\n> > diff --git a/git-send-email.perl b/git-send-email.perl\n> > index 5f92c89c1c1b..c1638d06f81d 100755\n> > --- a/git-send-email.perl\n> > +++ b/git-send-email.perl\n> > @@ -688,6 +688,9 @@ sub is_format_patch_arg {\n> > @files = handle_backup_files(@files);\n> >\n> > if ($validate) {\n> > + my %seen;\n> > + my @dupes = grep { $seen{$_}++ } @files;\n> > +\n> > foreach my $f (@files) {\n> > unless (-p $f) {\n> > my $error = validate_patch($f, $target_xfer_encoding);\n> > @@ -695,6 +698,17 @@ sub is_format_patch_arg {\n> > $f, $error);\n> > }\n>\n> This is not a fault of your patch at all, but the two hunks are\n> curious. If \"git format-patch\" chose to coalesce these two hunks\n> into one, the second hunk header can be replaced by\n>\n> $error and die sprintf(__(\"fatal: ...\"),\n>\n> The end result would be that we will spend the same number of lines\n> and we will see more useful information.\n>\n> > }\n> > + if (@dupes) {\n> > + printf(__(\"Patches specified several times: \\n\"));\n> > + printf(__(\"%s \\n\" x @dupes), @dupes);\n> > + $_ = ask(__(\"Do you want to send those patches several times? Y/n \"),\n> > + default => \"y\",\n> > + valid_re => qr/^(?:yes|y|no|n)/i);\n> > + if (/^n/i) {\n> > + cleanup_compose_files();\n> > + exit(0);\n> > + }\n> > + }\n>\n> Perhaps this should be inserted _before_ the \"let's examine each\n> patchfile and complain\" loop. Otherwise, you'd see this warning\n> only after seeing the same \"the file has too long a line\" error\n> on the same patch.\n>\n> While you are counting with %seen how many times the contents of\n> @files appear, perhaps you can also create a list of unique files,\n> so that you do not have to call validate_patch() more than once\n> for each of them. It would also allow you to offer another choice\n> in the above question \"do you want to send them more than once?\",\n> which may be much more useful than yes/no: \"drop duplicates\". If\n> you did so, you do not need to swap the order of the checks. You\n> would first count the occurences of each element in @files, then\n> call validate_patch() on each of them just once, and after you are\n> done, check if the user wants to send duplicates, abort, or dedup.\n>\n> Perhaps like this:\n>\n> if ($validate) {\n> + my (@dupes, %seen, @uniq);\n> +\n> foreach my $f (@files) {\n> + if ($seen{$f}) {\n> + if ($seen{$f} == 1) { push @dupes, $f; }\n> + next;\n> + }\n> + $seen{$f}++;\n> + push @uniq, $f;\n> + }\n> + foreach my $f (@uniq) {\n> unless (-p $f) {\n> my $error = validate_patch(...);\n> ... the existing loop ...\n> }\n> +\n> + if (@dupes) {\n> + ... the new code in your patch ...\n>\n"},{"id":"379626","messageId":"5775af07-f425-8192-3f2b-a45f21981407@arista.com","threadId":"51559","inReplyTo":"xmqq5znjrx0h.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCHv3] send-email: Ask if a patch should be sent twice","fromName":"Dmitry Safonov","fromEmail":"dima@arista.com","sentAt":"2019-07-30T23:05:39Z","receivedAt":"2019-07-30T23:05:45Z","isPatch":false,"sender":{"key":"dima@arista.com","avatar":null},"body":"Hi Junio,\n\nOn 7/30/19 11:13 PM, Junio C Hamano wrote:\n> Dmitry Safonov writes:\n> \n>> I was almost certain that git won't let me send the same patch twice,\n> \n> Why? And more importantly, does it matter to readers of this\n> message what you thought?\n\nI see the point of putting description in impersonal, technical way.\nProbably, was irritated after sending 50 patches instead of 37.\nAlso I've seen that in `git log | grep ' I '` over the project,\nnot that it justifies using it in the discussed patch.\n\n> \n>> but today I've managed to double-send a directory by a mistake:\n>> \tgit send-email --to linux-kernel@vger.kernel.org /tmp/timens/\n>> \t --cc 'Dmitry Safonov <0x7f454c46@gmail.com>' /tmp/timens/`\n>>\n>> [I haven't noticed that I put the directory twice ^^]\n>>\n>> Prevent this shipwreck from happening again by asking if a patch\n>> is sent multiple times on purpose.\n>>\n>> link: https://lkml.kernel.org/r/4d53ebc7-d5b2-346e-c383-606401d19d3a@gmail.com\n> \n> What does \"link:\" mean?\n\nIt's a tag, usually used to point URL for more information.\nMaybe not that common in git-ml, but often it shows an example of what\ncan go wrong without a patch.\n\n> \n>> Cc: Andrei Vagin \n> \n> What's the significance for this project to record that this patch\n> was CCed to Andrei?\n\nI've felt like I need to give credit to my friend with whom I was\nworking on the patches set; which I've failed to send properly.\nMaybe it's a thing of more opensource-friendly communities like lkml.\n\n> \n>> Suggested-by: Ævar Arnfjörð Bjarmason \n> \n> I think \"Helped-by:\" is a lot more appropriate, viewing the exchange\n> on v2 from the sideline.\n> \n>> Signed-off-by: Dmitry Safonov \n\nTo be honest, I don't feel like going on with v5.\nI've underestimate how welcoming some communities are to foreign patches.\n\nIf you feel like it's still worth to have the feature, please carry-on\nwith the tag\nAbandoned-by: Dmitry Safonov \nand don't drop Cc tag.\n\n[..]\nThanks and sorry for waisted time,\n Dmitry\n"},{"id":"379629","messageId":"20190731010625.GB9610@sigill.intra.peff.net","threadId":"51559","inReplyTo":"CANaxB-yygbxt=-sP+cyhR9WhcyEck+Wy=453huwfVNC9+So0BA@mail.gmail.com","subject":"Re: [PATCHv3] send-email: Ask if a patch should be sent twice","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-07-31T01:06:25Z","receivedAt":"2019-07-31T01:06:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 30, 2019 at 03:56:43PM -0700, Andrey Vagin wrote:\n\n> On Tue, Jul 30, 2019 at 3:13 PM Junio C Hamano wrote:\n> >\n> > Dmitry Safonov writes:\n> >\n> > > I was almost certain that git won't let me send the same patch twice,\n> >\n> > Why? And more importantly, does it matter to readers of this\n> > message what you thought?\n> \n> Sounds rude. What matter to readers except author's thoughts? I guess you want\n> to say that the comment should be in more neutral technical form without\n> personal pronouns.\n\nIMHO it's OK to use personal pronouns, but it's good to stick to the\nfacts. It's easy to go off on a tangent about what led you to the patch\nthat ends up making it harder for somebody reading the patch later to\nget to the point.\n\nIn this case, you did the leg-work to advance the story from \"I'm\nconfused about it should do...\" to \"it definitely does X now, and it\nwould be better if it did Y\". That's probably the best place for a later\nreader of the commit message to pick it up.\n\n-Peff\n\nPS I agree that what Junio wrote does sound a bit rude, but I don't\n think that was his intent. It's easy to accidentally be a little too\n curt during reviews (and I'm sure I've done it myself many times).\n Hopefully we can all take this as a reminder to be careful. :)\n"}]}