{"thread":{"id":"51627","subject":"[PATCH] t/perf: rename duplicate-numbered test script","startedAt":"2019-08-12T15:58:06Z","lastAt":"2019-08-12T16:13:54Z","messageCount":3,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"380310","messageId":"20190812155803.GA25161@sigill.intra.peff.net","threadId":"51627","inReplyTo":null,"subject":"[PATCH] t/perf: rename duplicate-numbered test script","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-08-12T15:58:03Z","receivedAt":"2019-08-12T15:58:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There are two perf scripts numbered p5600, but with otherwise different\nnames (\"clone-reference\" versus \"partial-clone\"). We store timing\nresults in files named after the whole script, so internally we don't\nget confused between the two. But \"aggregate.perl\" just prints the test\nnumber for each result, giving multiple entries for \"5600.3\". It also\nmakes it impossible to skip one test but not the other with\nGIT_SKIP_TESTS.\n\nLet's renumber the one that appeared later (by date -- the source of the\nproblem is that the two were developed on independent branches). For the\nnon-perf test suite, our test-lint rule would have complained about this\nwhen the two were merged, but t/perf never learned that trick.\n\nSigned-off-by: Jeff King \n---\nThis is meant for 2.23, but obviously it's not hurting anything if it\ndoesn't make the cut. I double-checked that there is no conflict with\nanything on pu, either. :)\n\nIn other news, I ran the whole perf suite on v2.23.0-rc2, and there was\nnothing interesting aside from the expected improvement from 39b44ba771\n(check_everything_connected: assume alternate ref tips are valid,\n2019-07-01).\n\n t/perf/{p5600-clone-reference.sh => p5601-clone-reference.sh} | 0\n 1 file changed, 0 insertions(+), 0 deletions(-)\n rename t/perf/{p5600-clone-reference.sh => p5601-clone-reference.sh} (100%)\n\ndiff --git a/t/perf/p5600-clone-reference.sh b/t/perf/p5601-clone-reference.sh\nsimilarity index 100%\nrename from t/perf/p5600-clone-reference.sh\nrename to t/perf/p5601-clone-reference.sh\n-- \n2.23.0.rc2.466.gd4688353c7\n"},{"id":"380311","messageId":"xmqqftm6bca3.fsf@gitster-ct.c.googlers.com","threadId":"51627","inReplyTo":"20190812155803.GA25161@sigill.intra.peff.net","subject":"Re: [PATCH] t/perf: rename duplicate-numbered test script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-12T16:04:36Z","receivedAt":"2019-08-12T16:04:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> There are two perf scripts numbered p5600, but with otherwise different\n> names (\"clone-reference\" versus \"partial-clone\"). We store timing\n> results in files named after the whole script, so internally we don't\n> get confused between the two. But \"aggregate.perl\" just prints the test\n> number for each result, giving multiple entries for \"5600.3\". It also\n> makes it impossible to skip one test but not the other with\n> GIT_SKIP_TESTS.\n>\n> Let's renumber the one that appeared later (by date -- the source of the\n> problem is that the two were developed on independent branches). For the\n> non-perf test suite, our test-lint rule would have complained about this\n> when the two were merged, but t/perf never learned that trick.\n>\n> Signed-off-by: Jeff King \n> ---\n> This is meant for 2.23, but obviously it's not hurting anything if it\n> doesn't make the cut. I double-checked that there is no conflict with\n> anything on pu, either. :)\n\nThanks for being careful. Will apply.\n\n> t/perf/{p5600-clone-reference.sh => p5601-clone-reference.sh} | 0\n> 1 file changed, 0 insertions(+), 0 deletions(-)\n> rename t/perf/{p5600-clone-reference.sh => p5601-clone-reference.sh} (100%)\n>\n> diff --git a/t/perf/p5600-clone-reference.sh b/t/perf/p5601-clone-reference.sh\n> similarity index 100%\n> rename from t/perf/p5600-clone-reference.sh\n> rename to t/perf/p5601-clone-reference.sh\n\nBy the way, do we feel differently (e.g. more risky) when we see\n100% rename without the \"index old-oid..new-oid mode\" lines and when\nwe see 99% rename with one, with a one-line change?\n"},{"id":"380313","messageId":"20190812161352.GB25388@sigill.intra.peff.net","threadId":"51627","inReplyTo":"xmqqftm6bca3.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] t/perf: rename duplicate-numbered test script","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-08-12T16:13:52Z","receivedAt":"2019-08-12T16:13:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 12, 2019 at 09:04:36AM -0700, Junio C Hamano wrote:\n\n> > t/perf/{p5600-clone-reference.sh => p5601-clone-reference.sh} | 0\n> > 1 file changed, 0 insertions(+), 0 deletions(-)\n> > rename t/perf/{p5600-clone-reference.sh => p5601-clone-reference.sh} (100%)\n> >\n> > diff --git a/t/perf/p5600-clone-reference.sh b/t/perf/p5601-clone-reference.sh\n> > similarity index 100%\n> > rename from t/perf/p5600-clone-reference.sh\n> > rename to t/perf/p5601-clone-reference.sh\n> \n> By the way, do we feel differently (e.g. more risky) when we see\n> 100% rename without the \"index old-oid..new-oid mode\" lines and when\n> we see 99% rename with one, with a one-line change?\n\nI saw that earlier message from Linus, too. :)\n\nFor a change like this, I don't think it matters either way. Whatever is\nthe content of that file, my intent is to move it to a new location. So\nif you did have changes, moving them along with it would be the right\nthing.\n\nThat said, I'm not at all opposed to having more data in the patch. Even\nif the \"apply\" side doesn't do anything useful with the \"index\" line in\nsuch a case, it's possible it could help with tracking down a mis-merge\nor other confusion after the fact.\n\nWhat I don't think we would want, though, is for a change on your side\nto reject a patch like mine (a sort of \"RENAME/MODIFY\" conflict, I\nguess, where \"am\" says \"I can't move 1234abcd to this new filename,\nbecause that's not what I have at that path\"). Our merges are already\nhappy to port changes around when there's a rename on another branch,\nand this case is no different.\n\n-Peff\n"}]}