{"thread":{"id":"38008","subject":"[RFD/PATCH] add: ignore only ignored files","startedAt":"2014-11-19T14:52:33Z","lastAt":"2014-11-25T03:57:08Z","messageCount":22,"participants":["Michael J Gruber","Junio C Hamano","Jeff King","Torsten Bögershausen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"252180","messageId":"3f78d6c1e35c87049daaac6cb1257ea8310a90bb.1416408015.git.git@drmicha.warpmail.net","threadId":"38008","inReplyTo":null,"subject":"[RFD/PATCH] add: ignore only ignored files","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2014-11-19T14:52:33Z","receivedAt":"2014-11-19T14:52:33Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"\"git add foo bar\" adds neither foo nor bar when bar is ignored, but dies\nto let the user recheck their command invocation. This becomes less\nhelpful when \"git add foo.*\" is subject to shell expansion and some of\nthe expanded files are ignored.\n\n\"git add --ignore-errors\" is supposed to ignore errors when indexing\nsome files and adds the others. It does ignore errors from actual\nindexing attempts, but does not ignore the error \"file is ignored\" as\noutlined above.\n\nChange \"git add --ignore-errors foo bar\" to add foo when bar is ignored,\ni.e. to ignore the ignore error.\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\nThus buggs me whenever I add a new TeX project when there are *.aux and\n*.log around also[*]. Aside from that personal itch: For a user reading the\ndocumentation, the difference between \"thinking about indexing a file\"\nand \"technically indexing a file\" is probably irrelevant, and most would\nexpect \"git add --ignore-errors\" to ignore the \"file is ignored\" error\nas well and continue adding the remaining files.\n\nThis is a behaviour change, though (albeit only for the option/config case),\nso it's RFD. If the direction is OK a test would be in order.\n\nWe could add yet another ignore option, of course, which just screams to be called\n--ignore-ignored, along with a config add.ignoreIgnored. I dunno...\n\n[*] And who wants to quote their *, really? ;)\n\n builtin/add.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ae6d3e2..8cadb71 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -284,7 +284,9 @@ static int add_files(struct dir_struct *dir, int flags)\n \t\tfor (i = 0; i < dir->ignored_nr; i++)\n \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n \t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n-\t\tdie(_(\"no files added\"));\n+\t\tif (!ignore_add_errors)\n+\t\t\tdie(_(\"no files added\"));\n+\t\texit_status = 1;\n \t}\n \n \tfor (i = 0; i < dir->nr; i++)\n-- \n2.2.0.rc2.293.gc05a35d\n"},{"id":"252193","messageId":"xmqqioibrplb.fsf@gitster.dls.corp.google.com","threadId":"38008","inReplyTo":"3f78d6c1e35c87049daaac6cb1257ea8310a90bb.1416408015.git.git@drmicha.warpmail.net","subject":"Re: [RFD/PATCH] add: ignore only ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-19T18:51:28Z","receivedAt":"2014-11-19T18:51:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n> ... and most would\n> expect \"git add --ignore-errors\" to ignore the \"file is ignored\" error\n> as well and continue adding the remaining files.\n\nYeah, I think that makes sense (but please don't take it as the final\ndecision yet---I'd like to hear from others).\n"},{"id":"252197","messageId":"20141119191502.GC9908@peff.net","threadId":"38008","inReplyTo":"3f78d6c1e35c87049daaac6cb1257ea8310a90bb.1416408015.git.git@drmicha.warpmail.net","subject":"Re: [RFD/PATCH] add: ignore only ignored files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-11-19T19:15:02Z","receivedAt":"2014-11-19T19:15:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 19, 2014 at 03:52:33PM +0100, Michael J Gruber wrote:\n\n> \"git add foo bar\" adds neither foo nor bar when bar is ignored, but dies\n> to let the user recheck their command invocation. This becomes less\n> helpful when \"git add foo.*\" is subject to shell expansion and some of\n> the expanded files are ignored.\n> \n> \"git add --ignore-errors\" is supposed to ignore errors when indexing\n> some files and adds the others. It does ignore errors from actual\n> indexing attempts, but does not ignore the error \"file is ignored\" as\n> outlined above.\n> \n> Change \"git add --ignore-errors foo bar\" to add foo when bar is ignored,\n> i.e. to ignore the ignore error.\n> \n> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n\nFWIW, your complaint and this new behavior makes sense to me.\n\nTypically I keep a very neat .gitignore file and just use \"git add .\",\nwhich _does_ ignore those files. The real problem here is that git\ncannot tell the difference between \"the user explicitly asked for\nfoo.aux, so we should complain\" and \"oops, foo.aux got caught in a shell\nexpansion\".\n\nI almost wonder if skipping ignored files should _always_ be a warning,\nnot a hard error. I guess that has unpleasant side effects for scripts\nwhich call \"git add XXX\" and check the exit code, who may be\nunpleasantly surprised that they missed out on some content.\n\nPerhaps we could do a hybrid: add the files that were not ignored, but\nthen still exit non-zero. Careful scripts need to check the exit status\nof \"git add\" anyway, and sloppy humans with over-broad wildcards\ntypically do not care about the exit status.\n\n-Peff\n"},{"id":"252216","messageId":"xmqqbno2rhlz.fsf@gitster.dls.corp.google.com","threadId":"38008","inReplyTo":"20141119191502.GC9908@peff.net","subject":"Re: [RFD/PATCH] add: ignore only ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-19T21:43:52Z","receivedAt":"2014-11-19T21:43:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Typically I keep a very neat .gitignore file and just use \"git add .\",\n> which _does_ ignore those files. The real problem here is that git\n> cannot tell the difference between \"the user explicitly asked for\n> foo.aux, so we should complain\" and \"oops, foo.aux got caught in a shell\n> expansion\".\n\nYup.  I also find myself doing \"git cmd -- \\*.ext\" to let Git, not\nmy shell, handle the patterns.\n\n> I almost wonder if skipping ignored files should _always_ be a warning,\n> not a hard error. I guess that has unpleasant side effects for scripts\n> which call \"git add XXX\" and check the exit code, who may be\n> unpleasantly surprised that they missed out on some content.\n>\n> Perhaps we could do a hybrid: add the files that were not ignored, but\n> then still exit non-zero. Careful scripts need to check the exit status\n> of \"git add\" anyway, and sloppy humans with over-broad wildcards\n> typically do not care about the exit status.\n\n;-)\n"},{"id":"252247","messageId":"546DB778.2000000@drmicha.warpmail.net","threadId":"38008","inReplyTo":"xmqqbno2rhlz.fsf@gitster.dls.corp.google.com","subject":"Re: [RFD/PATCH] add: ignore only ignored files","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2014-11-20T09:42:16Z","receivedAt":"2014-11-20T09:42:16Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano schrieb am 19.11.2014 um 22:43:\n> Jeff King <peff@peff.net> writes:\n> \n>> Typically I keep a very neat .gitignore file and just use \"git add .\",\n>> which _does_ ignore those files. The real problem here is that git\n>> cannot tell the difference between \"the user explicitly asked for\n>> foo.aux, so we should complain\" and \"oops, foo.aux got caught in a shell\n>> expansion\".\n> \n> Yup.  I also find myself doing \"git cmd -- \\*.ext\" to let Git, not\n> my shell, handle the patterns.\n\nThat is the correct way, of course.\n\n>> I almost wonder if skipping ignored files should _always_ be a warning,\n>> not a hard error. I guess that has unpleasant side effects for scripts\n>> which call \"git add XXX\" and check the exit code, who may be\n>> unpleasantly surprised that they missed out on some content.\n>>\n>> Perhaps we could do a hybrid: add the files that were not ignored, but\n>> then still exit non-zero. Careful scripts need to check the exit status\n>> of \"git add\" anyway, and sloppy humans with over-broad wildcards\n>> typically do not care about the exit status.\n> \n> ;-)\n> \n\nYou can simply say \"Michael\" in your last subclause above :)\n\nI'm wondering whether that behaviour change (without --ignore-errors) is\nOK - I don't mind, but hey, I usually don't.\n\nI think it all comes down to the fact whether specifying an ignored file\non the command line is considered an error or only \"possibly a user\nerror\" we should dwim around. \"git add\" being plumbing, I'm somehow\nhesitant to change behaviour unless git is asked to ignore error. And if\nI'm hesitant already... oh wait. git add is declared porcelain? I would\nnot have guessed without looking it up.\n\nMichael\n"},{"id":"252261","messageId":"20141120155621.GA30273@peff.net","threadId":"38008","inReplyTo":"546DB778.2000000@drmicha.warpmail.net","subject":"Re: [RFD/PATCH] add: ignore only ignored files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-11-20T15:56:21Z","receivedAt":"2014-11-20T15:56:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 20, 2014 at 10:42:16AM +0100, Michael J Gruber wrote:\n\n> >> Perhaps we could do a hybrid: add the files that were not ignored, but\n> >> then still exit non-zero. Careful scripts need to check the exit status\n> >> of \"git add\" anyway, and sloppy humans with over-broad wildcards\n> >> typically do not care about the exit status.\n> > \n> > ;-)\n> > \n> \n> You can simply say \"Michael\" in your last subclause above :)\n> \n> I'm wondering whether that behaviour change (without --ignore-errors) is\n> OK - I don't mind, but hey, I usually don't.\n\nI can't think of a case that it really hurts, but then I have not\nthought too hard on it. If you want to play with it, I think the patch\nis as simple as:\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ae6d3e2..1074e32 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -284,7 +284,7 @@ static int add_files(struct dir_struct *dir, int flags)\n \t\tfor (i = 0; i < dir->ignored_nr; i++)\n \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n \t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n-\t\tdie(_(\"no files added\"));\n+\t\texit_status = 1;\n \t}\n \n \tfor (i = 0; i < dir->nr; i++)\n\nIt needs a tweak to t3700.35, which expects the \"fatal:\" line on stderr.\nBut other than that, it passes all tests. So it must be good, right? :)\n\n-Peff\n"},{"id":"252266","messageId":"xmqqegsx4whi.fsf@gitster.dls.corp.google.com","threadId":"38008","inReplyTo":"20141120155621.GA30273@peff.net","subject":"Re: [RFD/PATCH] add: ignore only ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-20T17:23:21Z","receivedAt":"2014-11-20T17:23:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Nov 20, 2014 at 10:42:16AM +0100, Michael J Gruber wrote:\n>\n>> >> Perhaps we could do a hybrid: add the files that were not ignored, but\n>> >> then still exit non-zero. Careful scripts need to check the exit status\n>> >> of \"git add\" anyway, and sloppy humans with over-broad wildcards\n>> >> typically do not care about the exit status.\n>> > \n>> > ;-)\n>> > \n>> \n>> You can simply say \"Michael\" in your last subclause above :)\n>> \n>> I'm wondering whether that behaviour change (without --ignore-errors) is\n>> OK - I don't mind, but hey, I usually don't.\n>\n> I can't think of a case that it really hurts, but then I have not\n> thought too hard on it. If you want to play with it, I think the patch\n> is as simple as:\n>\n> diff --git a/builtin/add.c b/builtin/add.c\n> index ae6d3e2..1074e32 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -284,7 +284,7 @@ static int add_files(struct dir_struct *dir, int flags)\n>  \t\tfor (i = 0; i < dir->ignored_nr; i++)\n>  \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n>  \t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n> -\t\tdie(_(\"no files added\"));\n> +\t\texit_status = 1;\n>  \t}\n>  \n>  \tfor (i = 0; i < dir->nr; i++)\n>\n> It needs a tweak to t3700.35, which expects the \"fatal:\" line on stderr.\n> But other than that, it passes all tests. So it must be good, right? :)\n\n;-)\n\nIt indeed is a behaviour change, but I do not expect it would be too\nheavy a change to require us a transition period or major version\nbump.  But because that is just my expectation, which is not what\nreal world users would expect, so I'd prefer to cook such a change\nfor at least a cycle and a half in 'next'.\n"},{"id":"252278","messageId":"20141120182012.GD2559@peff.net","threadId":"38008","inReplyTo":"xmqqegsx4whi.fsf@gitster.dls.corp.google.com","subject":"Re: [RFD/PATCH] add: ignore only ignored files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-11-20T18:20:13Z","receivedAt":"2014-11-20T18:20:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 20, 2014 at 09:23:21AM -0800, Junio C Hamano wrote:\n\n> > diff --git a/builtin/add.c b/builtin/add.c\n> > index ae6d3e2..1074e32 100644\n> > --- a/builtin/add.c\n> > +++ b/builtin/add.c\n> > @@ -284,7 +284,7 @@ static int add_files(struct dir_struct *dir, int flags)\n> >  \t\tfor (i = 0; i < dir->ignored_nr; i++)\n> >  \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n> >  \t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n> > -\t\tdie(_(\"no files added\"));\n> > +\t\texit_status = 1;\n> >  \t}\n> >  \n> >  \tfor (i = 0; i < dir->nr; i++)\n> >\n> > It needs a tweak to t3700.35, which expects the \"fatal:\" line on stderr.\n> > But other than that, it passes all tests. So it must be good, right? :)\n> \n> ;-)\n> \n> It indeed is a behaviour change, but I do not expect it would be too\n> heavy a change to require us a transition period or major version\n> bump.  But because that is just my expectation, which is not what\n> real world users would expect, so I'd prefer to cook such a change\n> for at least a cycle and a half in 'next'.\n\nOh, definitely. Showing the patch at all was my not-so-subtle attempt to\nconvince Michael to take over the topic so I did not have to worry about\nsuch things. :)\n\n-Peff\n"},{"id":"252343","messageId":"546F5CC7.8060904@drmicha.warpmail.net","threadId":"38008","inReplyTo":"20141120182012.GD2559@peff.net","subject":"Re: [RFD/PATCH] add: ignore only ignored files","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2014-11-21T15:39:51Z","receivedAt":"2014-11-21T15:39:51Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King schrieb am 20.11.2014 um 19:20:\n> On Thu, Nov 20, 2014 at 09:23:21AM -0800, Junio C Hamano wrote:\n> \n>>> diff --git a/builtin/add.c b/builtin/add.c\n>>> index ae6d3e2..1074e32 100644\n>>> --- a/builtin/add.c\n>>> +++ b/builtin/add.c\n>>> @@ -284,7 +284,7 @@ static int add_files(struct dir_struct *dir, int flags)\n>>>  \t\tfor (i = 0; i < dir->ignored_nr; i++)\n>>>  \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n>>>  \t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n>>> -\t\tdie(_(\"no files added\"));\n>>> +\t\texit_status = 1;\n>>>  \t}\n>>>  \n>>>  \tfor (i = 0; i < dir->nr; i++)\n>>>\n>>> It needs a tweak to t3700.35, which expects the \"fatal:\" line on stderr.\n>>> But other than that, it passes all tests. So it must be good, right? :)\n>>\n>> ;-)\n>>\n>> It indeed is a behaviour change, but I do not expect it would be too\n>> heavy a change to require us a transition period or major version\n>> bump.  But because that is just my expectation, which is not what\n>> real world users would expect, so I'd prefer to cook such a change\n>> for at least a cycle and a half in 'next'.\n> \n> Oh, definitely. Showing the patch at all was my not-so-subtle attempt to\n> convince Michael to take over the topic so I did not have to worry about\n> such things. :)\n\nWell, given that I figured out how to do it with the option, it may come\nat no additional surprise that I figured out how to do it without the\noption, as well.\n\nThe real extra surprise is - not only that test fix is in my tree\nalready - it even comes with a test that actually tests the new intended\nbehavior. Whoaaa!\n\nI'll resend it soon so that Jeff can take over and adjust the commit\nmessage ;)\n\nMichael, just kidding\n"},{"id":"252346","messageId":"b4834f562679d7ccad683463edc61db5ea962d8d.1416585536.git.git@drmicha.warpmail.net","threadId":"38008","inReplyTo":"546F5CC7.8060904@drmicha.warpmail.net","subject":"[PATCHv2] add: ignore only ignored files","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2014-11-21T16:08:19Z","receivedAt":"2014-11-21T16:08:19Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"\"git add foo bar\" adds neither foo nor bar when bar is ignored, but dies\nto let the user recheck their command invocation. This becomes less\nhelpful when \"git add foo.*\" is subject to shell expansion and some of\nthe expanded files are ignored.\n\n\"git add --ignore-errors\" is supposed to ignore errors when indexing\nsome files and adds the others. It does ignore errors from actual\nindexing attempts, but does not ignore the error \"file is ignored\" as\noutlined above. This is unexpected.\n\nChange \"git add foo bar\" to add foo when bar is ignored, but issue\na warning and return a failure code as before the change.\n\nThat is, in the case of trying to add ignored files we now act the same\nway (with or without \"--ignore-errors\") in which we act for more\nsevere indexing errors when \"--ignore-errors\" is specified.\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\nAs discussed, we change behavior even without the option.\nI.a.w.: We declare \"add ignored file\" as a minor error compared\nto an actual indexing error.\n\nMy sincere thanks go out to Jeff without whom I could not possibly\nhave come up with a patch like this :)\n\n builtin/add.c  | 2 +-\n t/t3700-add.sh | 8 +++++++-\n 2 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ae6d3e2..1074e32 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -284,7 +284,7 @@ static int add_files(struct dir_struct *dir, int flags)\n \t\tfor (i = 0; i < dir->ignored_nr; i++)\n \t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n \t\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n-\t\tdie(_(\"no files added\"));\n+\t\texit_status = 1;\n \t}\n \n \tfor (i = 0; i < dir->nr; i++)\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex fe274e2..f7ff1f5 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -91,6 +91,13 @@ test_expect_success 'error out when attempting to add ignored ones without -f' '\n \t! (git ls-files | grep \"\\\\.ig\")\n '\n \n+test_expect_success 'error out when attempting to add ignored ones but add others' '\n+\ttouch a.if &&\n+\ttest_must_fail git add a.?? &&\n+\t! (git ls-files | grep \"\\\\.ig\") &&\n+\t(git ls-files | grep a.if)\n+'\n+\n test_expect_success 'add ignored ones with -f' '\n \tgit add -f a.?? &&\n \tgit ls-files --error-unmatch a.ig\n@@ -311,7 +318,6 @@ cat >expect.err <<\\EOF\n The following paths are ignored by one of your .gitignore files:\n ignored-file\n Use -f if you really want to add them.\n-fatal: no files added\n EOF\n cat >expect.out <<\\EOF\n add 'track-this'\n-- \n2.2.0.rc2.293.gc05a35d\n"},{"id":"252354","messageId":"20141121180105.GB26650@peff.net","threadId":"38008","inReplyTo":"b4834f562679d7ccad683463edc61db5ea962d8d.1416585536.git.git@drmicha.warpmail.net","subject":"Re: [PATCHv2] add: ignore only ignored files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-11-21T18:01:05Z","receivedAt":"2014-11-21T18:01:05Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 21, 2014 at 05:08:19PM +0100, Michael J Gruber wrote:\n\n> \"git add foo bar\" adds neither foo nor bar when bar is ignored, but dies\n> to let the user recheck their command invocation. This becomes less\n> helpful when \"git add foo.*\" is subject to shell expansion and some of\n> the expanded files are ignored.\n> \n> \"git add --ignore-errors\" is supposed to ignore errors when indexing\n> some files and adds the others. It does ignore errors from actual\n> indexing attempts, but does not ignore the error \"file is ignored\" as\n> outlined above. This is unexpected.\n> \n> Change \"git add foo bar\" to add foo when bar is ignored, but issue\n> a warning and return a failure code as before the change.\n> \n> That is, in the case of trying to add ignored files we now act the same\n> way (with or without \"--ignore-errors\") in which we act for more\n> severe indexing errors when \"--ignore-errors\" is specified.\n\nThanks, this looks pretty good to me. I agree with Junio's sense that we\nshould cook it extra long to give people time to react.\n\n> My sincere thanks go out to Jeff without whom I could not possibly\n> have come up with a patch like this :)\n\n:) Sorry if I was being obnoxious before. Sometimes contributors need a\ngentle push to keep going, but I should know by now that you are not\nsuch a person.\n\n> diff --git a/t/t3700-add.sh b/t/t3700-add.sh\n> index fe274e2..f7ff1f5 100755\n> --- a/t/t3700-add.sh\n> +++ b/t/t3700-add.sh\n> @@ -91,6 +91,13 @@ test_expect_success 'error out when attempting to add ignored ones without -f' '\n>  \t! (git ls-files | grep \"\\\\.ig\")\n>  '\n>  \n> +test_expect_success 'error out when attempting to add ignored ones but add others' '\n> +\ttouch a.if &&\n> +\ttest_must_fail git add a.?? &&\n> +\t! (git ls-files | grep \"\\\\.ig\") &&\n> +\t(git ls-files | grep a.if)\n> +'\n\nI am somewhat allergic to pipes in our test suite, because they can mask\nerrors (especially with a negated grep, because we do not know if they\ncorrectly produced any output at all). But I guess this is matching the\nsurrounding code, and it is quite unlikely for `ls-files` to fail in any\nmeaningful way here. So I think it's fine.\n\n-Peff\n"},{"id":"252376","messageId":"5470A4C0.3070501@web.de","threadId":"38008","inReplyTo":"20141121180105.GB26650@peff.net","subject":"Re: [PATCHv2] add: ignore only ignored files","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-11-22T14:59:12Z","receivedAt":"2014-11-22T14:59:12Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":">> +test_expect_success 'error out when attempting to add ignored ones but add others' '\n>> +\ttouch a.if &&\n>> +\ttest_must_fail git add a.?? &&\n>> +\t! (git ls-files | grep \"\\\\.ig\") &&\n>> +\t(git ls-files | grep a.if)\n>> +'\n> \n> I am somewhat allergic to pipes in our test suite, because they can mask\n> errors (especially with a negated grep, because we do not know if they\n> correctly produced any output at all). But I guess this is matching the\n> surrounding code, and it is quite unlikely for `ls-files` to fail in any\n> meaningful way here. So I think it's fine.\n> \n> -Peff\n\n2 small comments:\nWhy the escaped \"\\\\.ig\" and the unescaped \"a.if\"  ?\n\nThe other question, this is a more general one, strikes me every time I see\n! grep\n\nShould we avoid it by writing \"test_must_fail\" instead of \"!\" ?\n(The current code base has a mixture of both)\n\nThe following came into my mind when working on another grepy thing,\nand it may be unnecessary clumsy:\n\ntest_expect_success 'error out when attempting to add ignored ones but add others' '\n\ttouch a.if &&\n\ttest_must_fail git add a.?? &&\n\tgit ls-files >files.txt &&\n\ttest_must_fail grep a.ig files.txt >/dev/null &&\n\tgrep a.if files.txt >/dev/null &&\n\trm files.txt\n'\n\n(It feels as if there should be a \"grepnot\" ;-)\n\n\nThe 3rd comment:\nThanks for taking this up!\n"},{"id":"252379","messageId":"20141122191932.GA13340@peff.net","threadId":"38008","inReplyTo":"5470A4C0.3070501@web.de","subject":"Re: [PATCHv2] add: ignore only ignored files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-11-22T19:19:33Z","receivedAt":"2014-11-22T19:19:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 22, 2014 at 03:59:12PM +0100, Torsten Bögershausen wrote:\n\n> >> +test_expect_success 'error out when attempting to add ignored ones but add others' '\n> >> +\ttouch a.if &&\n> >> +\ttest_must_fail git add a.?? &&\n> >> +\t! (git ls-files | grep \"\\\\.ig\") &&\n> >> +\t(git ls-files | grep a.if)\n> >> +'\n> [...]\n> \n> 2 small comments:\n> Why the escaped \"\\\\.ig\" and the unescaped \"a.if\"  ?\n\nI agree that is inconsistent, and I don't see any reason for it.\n\n> The other question, this is a more general one, strikes me every time I see\n> ! grep\n> \n> Should we avoid it by writing \"test_must_fail\" instead of \"!\" ?\n\nNo. The point of test_must_fail over \"!\" is to check that not only does\nthe command fail, but it fails with a clean exit rather than a signal\ndeath.  The general philosophy is that this is useful for git (which we\nare testing), and not for third-party tools that we are using to check\nour outputs. In other words, we do not expect grep to segfault, and do\nnot need to bother checking it.\n\nI do not think there is a real _downside_ to using test_must_fail for\ngrep, except that it is a bit more verbose.\n\nAnd that describes the goal, of course; actual implementation has been\nless consistent. Possibly because I do not know that those instructions\nare written down anywhere. We usually catch such things in review these\ndays, but there are many inconsistent spots in the existing suite.\n\n> The following came into my mind when working on another grepy thing,\n> and it may be unnecessary clumsy:\n> \n> test_expect_success 'error out when attempting to add ignored ones but add others' '\n> \ttouch a.if &&\n> \ttest_must_fail git add a.?? &&\n> \tgit ls-files >files.txt &&\n> \ttest_must_fail grep a.ig files.txt >/dev/null &&\n> \tgrep a.if files.txt >/dev/null &&\n> \trm files.txt\n\nRight, my \"allergic to pipes\" was basically advocating using a tempfile.\nBut as noted above, it should remain \"! grep\" here. And there is no need\nto redirect the output of grep, as the test suite does it already (in\nfact, it is preferable not to, because somebody debugging the test with\n\"-v\" will get more output).\n\n-Peff\n"},{"id":"252387","messageId":"5470FE0A.1030802@web.de","threadId":"38008","inReplyTo":"20141122191932.GA13340@peff.net","subject":"Re: [PATCHv2] add: ignore only ignored files","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-11-22T21:20:10Z","receivedAt":"2014-11-22T21:20:10Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2014-11-22 20.19, Jeff King wrote:\n> On Sat, Nov 22, 2014 at 03:59:12PM +0100, Torsten Bögershausen wrote:\n> \n>>>> +test_expect_success 'error out when attempting to add ignored ones but add others' '\n>>>> +\ttouch a.if &&\n>>>> +\ttest_must_fail git add a.?? &&\n>>>> +\t! (git ls-files | grep \"\\\\.ig\") &&\n>>>> +\t(git ls-files | grep a.if)\n>>>> +'\n>> [...]\n>>\n>> 2 small comments:\n>> Why the escaped \"\\\\.ig\" and the unescaped \"a.if\"  ?\n> \n> I agree that is inconsistent, and I don't see any reason for it.\n> \n>> The other question, this is a more general one, strikes me every time I see\n>> ! grep\n>>\n>> Should we avoid it by writing \"test_must_fail\" instead of \"!\" ?\n> \n> No. The point of test_must_fail over \"!\" is to check that not only does\n> the command fail, but it fails with a clean exit rather than a signal\n> death.  The general philosophy is that this is useful for git (which we\n> are testing), and not for third-party tools that we are using to check\n> our outputs. In other words, we do not expect grep to segfault, and do\n> not need to bother checking it.\n> \n> I do not think there is a real _downside_ to using test_must_fail for\n> grep, except that it is a bit more verbose.\nWe may burn CPU cycles \n> \n> And that describes the goal, of course; actual implementation has been\n> less consistent. Possibly because I do not know that those instructions\n> are written down anywhere. \nThere is a hint in test-lib-functions.sh, but a short notice in CodingGuidelines\ncould be useful, once there is an agreement about grep, which I think we have. \n> We usually catch such things in review these\n> days, but there are many inconsistent spots in the existing suite.\n> \n>> The following came into my mind when working on another grepy thing,\n>> and it may be unnecessary clumsy:\n>>\n>> test_expect_success 'error out when attempting to add ignored ones but add others' '\n>> \ttouch a.if &&\n>> \ttest_must_fail git add a.?? &&\n>> \tgit ls-files >files.txt &&\n>> \ttest_must_fail grep a.ig files.txt >/dev/null &&\n>> \tgrep a.if files.txt >/dev/null &&\n>> \trm files.txt\n> \n> Right, my \"allergic to pipes\" was basically advocating using a tempfile.\n> But as noted above, it should remain \"! grep\" here. And there is no need\n> to redirect the output of grep, as the test suite does it already (in\n> fact, it is preferable not to, because somebody debugging the test with\n> \"-v\" will get more output).\n> \n> -Peff\n\nI counted 19 \"test_must_fail grep\" under t/*sh, and 201 \"! grep\".\n\nAs a general rule for further review of shell scripts can we say ?\n! git                # incorrect, we don't capture e.g. segfaults of signal \ntest_must_fail grep  # correct, but not preferred for new code\n! grep               # preferred for new code\ntest_must_fail git   # correct\n"},{"id":"252396","messageId":"xmqqvbm5yehk.fsf@gitster.dls.corp.google.com","threadId":"38008","inReplyTo":"20141122191932.GA13340@peff.net","subject":"Re: [PATCHv2] add: ignore only ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-23T18:10:47Z","receivedAt":"2014-11-23T18:10:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> ... Possibly because I do not know that those instructions\n> are written down anywhere. We usually catch such things in review these\n> days, but there are many inconsistent spots in the existing suite.\n\nt/README has this\n\n    Don't:\n\n     - use '! git cmd' when you want to make sure the git command exits\n       with failure in a controlled way by calling \"die()\".  Instead,\n       use 'test_must_fail git cmd'.  This will signal a failure if git\n       dies in an unexpected way (e.g. segfault).\n\n       On the other hand, don't use test_must_fail for running regular\n       platform commands; just use '! cmd'.\n\nThough it can be improved by justifying \"just use '! cmd'\" further\nwith a bit of rationale (e.g. \"We are not in the business of\nverifying that world given to us sanely works.\"), I think the\ncurrent text is sufficient.\n\nDo we refer to t/README from CodingGuidelines where we tell the\ndevelopers to always write tests to prevent other people from\nbreaking tomorrow what you did today?  If not, perhaps that is what\nneeds to be added.\n"},{"id":"252406","messageId":"20141123194624.GB16605@peff.net","threadId":"38008","inReplyTo":"xmqqvbm5yehk.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCHv2] add: ignore only ignored files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-11-23T19:46:25Z","receivedAt":"2014-11-23T19:46:25Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 23, 2014 at 10:10:47AM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > ... Possibly because I do not know that those instructions\n> > are written down anywhere. We usually catch such things in review these\n> > days, but there are many inconsistent spots in the existing suite.\n> \n> t/README has this\n> \n>     Don't:\n> \n>      - use '! git cmd' when you want to make sure the git command exits\n>        with failure in a controlled way by calling \"die()\".  Instead,\n>        use 'test_must_fail git cmd'.  This will signal a failure if git\n>        dies in an unexpected way (e.g. segfault).\n> \n>        On the other hand, don't use test_must_fail for running regular\n>        platform commands; just use '! cmd'.\n\nThanks, I did not actually look and relied on my memory, which was\nobviously wrong. I agree that the instructions there are sufficient.\n\n> Do we refer to t/README from CodingGuidelines where we tell the\n> developers to always write tests to prevent other people from\n> breaking tomorrow what you did today?  If not, perhaps that is what\n> needs to be added.\n\nThat might make sense. It might also be that Torsten simply overlooked\nit when asking his question (i.e., there is nothing to fix,\ndocumentation is not always read completely, and we can move on).\n\n-Peff\n"},{"id":"252407","messageId":"20141123195041.GA16854@peff.net","threadId":"38008","inReplyTo":"5470FE0A.1030802@web.de","subject":"Re: [PATCHv2] add: ignore only ignored files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-11-23T19:50:42Z","receivedAt":"2014-11-23T19:50:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 22, 2014 at 10:20:10PM +0100, Torsten Bögershausen wrote:\n\n> > I do not think there is a real _downside_ to using test_must_fail for\n> > grep, except that it is a bit more verbose.\n> We may burn CPU cycles \n\nIt adds a single if/else chain. If your shell does not implement that as\na fast builtin, you have bigger performance problems. :)\n\n> I counted 19 \"test_must_fail grep\" under t/*sh, and 201 \"! grep\".\n\nI do not mind a patch to fix them, but with the usual caveat of avoiding\nstepping on the toes of any topics in flight. It is also fine to leave\nthem until the area is touched.\n\n> As a general rule for further review of shell scripts can we say ?\n> ! git                # incorrect, we don't capture e.g. segfaults of signal \n> test_must_fail grep  # correct, but not preferred for new code\n> ! grep               # preferred for new code\n> test_must_fail git   # correct\n\nI think that's true. The snippet from t/README Junio quoted lays it out\npretty clearly, I think. If you didn't know there was documentation in\nt/README that was worth reading before writing tests, then that is the\nthing I think should go in CodingGuidelines.\n\n-Peff\n"},{"id":"252423","messageId":"54730735.1080500@drmicha.warpmail.net","threadId":"38008","inReplyTo":"20141121180105.GB26650@peff.net","subject":"Re: [PATCHv2] add: ignore only ignored files","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2014-11-24T10:23:49Z","receivedAt":"2014-11-24T10:23:49Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King schrieb am 21.11.2014 um 19:01:\n> On Fri, Nov 21, 2014 at 05:08:19PM +0100, Michael J Gruber wrote:\n> \n>> \"git add foo bar\" adds neither foo nor bar when bar is ignored, but dies\n>> to let the user recheck their command invocation. This becomes less\n>> helpful when \"git add foo.*\" is subject to shell expansion and some of\n>> the expanded files are ignored.\n>>\n>> \"git add --ignore-errors\" is supposed to ignore errors when indexing\n>> some files and adds the others. It does ignore errors from actual\n>> indexing attempts, but does not ignore the error \"file is ignored\" as\n>> outlined above. This is unexpected.\n>>\n>> Change \"git add foo bar\" to add foo when bar is ignored, but issue\n>> a warning and return a failure code as before the change.\n>>\n>> That is, in the case of trying to add ignored files we now act the same\n>> way (with or without \"--ignore-errors\") in which we act for more\n>> severe indexing errors when \"--ignore-errors\" is specified.\n> \n> Thanks, this looks pretty good to me. I agree with Junio's sense that we\n> should cook it extra long to give people time to react.\n> \n>> My sincere thanks go out to Jeff without whom I could not possibly\n>> have come up with a patch like this :)\n> \n> :) Sorry if I was being obnoxious before. Sometimes contributors need a\n> gentle push to keep going, but I should know by now that you are not\n> such a person.\n\nWe were just having fun with each other ;)\n\n>> diff --git a/t/t3700-add.sh b/t/t3700-add.sh\n>> index fe274e2..f7ff1f5 100755\n>> --- a/t/t3700-add.sh\n>> +++ b/t/t3700-add.sh\n>> @@ -91,6 +91,13 @@ test_expect_success 'error out when attempting to add ignored ones without -f' '\n>>  \t! (git ls-files | grep \"\\\\.ig\")\n>>  '\n>>  \n>> +test_expect_success 'error out when attempting to add ignored ones but add others' '\n>> +\ttouch a.if &&\n>> +\ttest_must_fail git add a.?? &&\n>> +\t! (git ls-files | grep \"\\\\.ig\") &&\n>> +\t(git ls-files | grep a.if)\n>> +'\n> \n> I am somewhat allergic to pipes in our test suite, because they can mask\n> errors (especially with a negated grep, because we do not know if they\n> correctly produced any output at all). But I guess this is matching the\n> surrounding code, and it is quite unlikely for `ls-files` to fail in any\n> meaningful way here. So I think it's fine.\n> \n> -Peff\n> \n\nI do prefer test_cmp myself, also because it tells you much more in case\nof a broken test - a failed boolean chain doesn't even tell you where it\nbroke.\n\nIn this specific case, many more tests would need to be rewriten,\nthough, so I preferred to keep the style of the surrounding code.\n\nMichael\n"},{"id":"252424","messageId":"54730897.3000507@drmicha.warpmail.net","threadId":"38008","inReplyTo":"5470A4C0.3070501@web.de","subject":"Re: [PATCHv2] add: ignore only ignored files","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2014-11-24T10:29:43Z","receivedAt":"2014-11-24T10:29:43Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Torsten Bögershausen schrieb am 22.11.2014 um 15:59:\n>>> +test_expect_success 'error out when attempting to add ignored ones but add others' '\n>>> +\ttouch a.if &&\n>>> +\ttest_must_fail git add a.?? &&\n>>> +\t! (git ls-files | grep \"\\\\.ig\") &&\n>>> +\t(git ls-files | grep a.if)\n>>> +'\n>>\n>> I am somewhat allergic to pipes in our test suite, because they can mask\n>> errors (especially with a negated grep, because we do not know if they\n>> correctly produced any output at all). But I guess this is matching the\n>> surrounding code, and it is quite unlikely for `ls-files` to fail in any\n>> meaningful way here. So I think it's fine.\n>>\n>> -Peff\n> \n> 2 small comments:\n> Why the escaped \"\\\\.ig\" and the unescaped \"a.if\"  ?\n\nWell, the first one is copied straight from another test and the second\none is by me. ;)\n\nI want to test that no files ending in .ig are added, but that one file\na.if is added. Knowing how the test is structured, it is higly unlikely\nthat other people will add a file where the dot in a.if matches\nsomething other than a dot, but in the case of .ig I wouldn't be so\nsure. We could take the extra safety measure and quote \"a\\\\.if\" also,\nbut to me that seems to make the test less readable.\n\n> The other question, this is a more general one, strikes me every time I see\n> ! grep\n> \n> Should we avoid it by writing \"test_must_fail\" instead of \"!\" ?\n> (The current code base has a mixture of both)\n> \n> The following came into my mind when working on another grepy thing,\n> and it may be unnecessary clumsy:\n> \n> test_expect_success 'error out when attempting to add ignored ones but add others' '\n> \ttouch a.if &&\n> \ttest_must_fail git add a.?? &&\n> \tgit ls-files >files.txt &&\n> \ttest_must_fail grep a.ig files.txt >/dev/null &&\n> \tgrep a.if files.txt >/dev/null &&\n> \trm files.txt\n> '\n> \n> (It feels as if there should be a \"grepnot\" ;-)\n> \n\nI guess that was clarified in the ongoing discussion.\n\n> The 3rd comment:\n> Thanks for taking this up!\n\nJust scratching my own itches mostly these days :)\n\nMichael\n"},{"id":"252434","messageId":"xmqqzjbgwl77.fsf@gitster.dls.corp.google.com","threadId":"38008","inReplyTo":"20141123194624.GB16605@peff.net","subject":"Re: [PATCHv2] add: ignore only ignored files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-24T17:41:00Z","receivedAt":"2014-11-24T17:41:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sun, Nov 23, 2014 at 10:10:47AM -0800, Junio C Hamano wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>> \n>> > ... Possibly because I do not know that those instructions\n>> > are written down anywhere. We usually catch such things in review these\n>> > days, but there are many inconsistent spots in the existing suite.\n>> \n>> t/README has this\n>> \n>>     Don't:\n>> \n>>      - use '! git cmd' when you want to make sure the git command exits\n>>        with failure in a controlled way by calling \"die()\".  Instead,\n>>        use 'test_must_fail git cmd'.  This will signal a failure if git\n>>        dies in an unexpected way (e.g. segfault).\n>> \n>>        On the other hand, don't use test_must_fail for running regular\n>>        platform commands; just use '! cmd'.\n>\n> Thanks, I did not actually look and relied on my memory, which was\n> obviously wrong. I agree that the instructions there are sufficient.\n>\n>> Do we refer to t/README from CodingGuidelines where we tell the\n>> developers to always write tests to prevent other people from\n>> breaking tomorrow what you did today?  If not, perhaps that is what\n>> needs to be added.\n>\n> That might make sense. It might also be that Torsten simply overlooked\n> it when asking his question (i.e., there is nothing to fix,\n> documentation is not always read completely, and we can move on).\n\nWe actually do not have a reference to it anywhere.  For now, this\nshould suffice.\n\n Documentation/SubmittingPatches | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\nindex fa71b5f..a3861a6 100644\n--- a/Documentation/SubmittingPatches\n+++ b/Documentation/SubmittingPatches\n@@ -57,7 +57,8 @@ change, the approach taken by the change, and if relevant how this\n differs substantially from the prior version, are all good things\n to have.\n \n-Make sure that you have tests for the bug you are fixing.\n+Make sure that you have tests for the bug you are fixing.  See\n+t/README for guidance of writing tests.\n \n When adding a new feature, make sure that you have new tests to show\n the feature triggers the new behaviour when it should, and to show the\n"},{"id":"252446","messageId":"5473937F.6000101@web.de","threadId":"38008","inReplyTo":"xmqqzjbgwl77.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCHv2] add: ignore only ignored files","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-11-24T20:22:23Z","receivedAt":"2014-11-24T20:22:23Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2014-11-24 18.41, Junio C Hamano wrote:\n...\n>>> Do we refer to t/README from CodingGuidelines where we tell the\n>>> developers to always write tests to prevent other people from\n>>> breaking tomorrow what you did today?  If not, perhaps that is what\n>>> needs to be added.\n>>\n>> That might make sense. It might also be that Torsten simply overlooked\n>> it when asking his question (i.e., there is nothing to fix,\n>> documentation is not always read completely, and we can move on).\n\nThanks, until yesterday I was unaware of t/README, but now I am :-)\n....\n"},{"id":"252489","messageId":"20141125035707.GB19301@peff.net","threadId":"38008","inReplyTo":"xmqqzjbgwl77.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCHv2] add: ignore only ignored files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-11-25T03:57:08Z","receivedAt":"2014-11-25T03:57:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 24, 2014 at 09:41:00AM -0800, Junio C Hamano wrote:\n\n> We actually do not have a reference to it anywhere.  For now, this\n> should suffice.\n> \n>  Documentation/SubmittingPatches | 3 ++-\n>  1 file changed, 2 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\n> index fa71b5f..a3861a6 100644\n> --- a/Documentation/SubmittingPatches\n> +++ b/Documentation/SubmittingPatches\n> @@ -57,7 +57,8 @@ change, the approach taken by the change, and if relevant how this\n>  differs substantially from the prior version, are all good things\n>  to have.\n>  \n> -Make sure that you have tests for the bug you are fixing.\n> +Make sure that you have tests for the bug you are fixing.  See\n> +t/README for guidance of writing tests.\n\nThat looks a good improvement to me.\n\n-Peff\n"}]}