{"thread":{"id":"1050","subject":"[PATCH 2/2] Fix for git-rev-list --merge-order B ^A (A,B share common base)","startedAt":"2005-06-29T23:45:33Z","lastAt":"2005-06-30T02:47:44Z","messageCount":4,"participants":["Jon Seymour","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"5430","messageId":"20050629234533.28709.qmail@blackcubes.dyndns.org","threadId":"1050","inReplyTo":null,"subject":"[PATCH 2/2] Fix for git-rev-list --merge-order B ^A (A,B share common base)","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2005-06-29T23:45:33Z","receivedAt":"2005-06-29T23:45:33Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"\nThis patch makes --merge-order produce the same list as git-rev-list \nwithout --merge-order specified.\n\nIn particular, if the graph looks like this:\n\nA\n| B\n|/ \nC\n|\nD\n\nThe both git-rev-list B ^A and git-rev-list --merge-order will produce B.\n\nThe unit test changes in this patch remove use of the --show-breaks \nflags from certain unit tests. The changed --merge-order behaviour \nchanged the annotation that --show-breaks prints for certain test cases. \nThe new behaviour is reasonable and irrelevant to the intent of the tests\nso that tests have been changed to eliminate the spurious behaviour.\n\nSigned-off-by: Jon Seymour \n---\n\n epoch.c | 8 +++-----\n t/t6001-rev-list-merge-order.sh | 36 ++++++++++++++++++------------------\n 2 files changed, 21 insertions(+), 23 deletions(-)\n\n26783f6660e58cc2e988b92050707a6a417cc6ae\ndiff --git a/epoch.c b/epoch.c\n--- a/epoch.c\n+++ b/epoch.c\n@@ -585,11 +585,9 @@ int sort_list_in_merge_order(struct comm\n \tfor (; list; list = list->next) {\n \t\tstruct commit *next = list->item;\n \n-\t\tif (!(next->object.flags & UNINTERESTING)) {\n-\t\t\tif (!(next->object.flags & DUPCHECK)) {\n-\t\t\t\tnext->object.flags |= DUPCHECK;\n-\t\t\t\tcommit_list_insert(list->item, &reversed);\n-\t\t\t}\n+\t\tif (!(next->object.flags & DUPCHECK)) {\n+\t\t\tnext->object.flags |= DUPCHECK;\n+\t\t\tcommit_list_insert(list->item, &reversed);\n \t\t}\n \t}\n \ndiff --git a/t/t6001-rev-list-merge-order.sh b/t/t6001-rev-list-merge-order.sh\n--- a/t/t6001-rev-list-merge-order.sh\n+++ b/t/t6001-rev-list-merge-order.sh\n@@ -366,34 +366,34 @@ test_output_expect_success \"three nodes \n = root\n EOF\n \n-test_output_expect_success \"linear prune l2 ^root\" 'git-rev-list --merge-order --show-breaks l2 ^root' <>>>> \"JS\" == Jon Seymour writes:\n\nI am puzzled about this part.\n\nJS> The unit test changes in this patch remove use of the --show-breaks \nJS> flags from certain unit tests. The changed --merge-order behaviour \nJS> changed the annotation that --show-breaks prints for certain test cases. \nJS> The new behaviour is reasonable and irrelevant to the intent of the tests\nJS> so that tests have been changed to eliminate the spurious behaviour.\n\nIf the behaviour of --show-breaks subtly changes, and if that\nchanged behaviour is something still acceptable, why not update\nthe test to show the new expected results since you are updating\nthe test anyway?\n\nShowing that \"subtle\" change in the diff may draw people's\nattention and would help you to verify that the behaviour change\nis not something that would be unacceptable to them.\n\nAlso if you are changing t6001, could you also merge Mark\nAllen's BSD portability fix while you are at it?\n\n Message-ID: <20050628014337.18986.qmail@web41205.mail.yahoo.com>\n"},{"id":"5432","messageId":"2cfc403205062918336a55e8da@mail.gmail.com","threadId":"1050","inReplyTo":"7v1x6k1z6c.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/2] Fix for git-rev-list --merge-order B ^A (A,B share common base)","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2005-06-30T01:33:25Z","receivedAt":"2005-06-30T01:33:25Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On 6/30/05, Junio C Hamano wrote:\n> >>>>> \"JS\" == Jon Seymour writes:\n> \n> I am puzzled about this part.\n> \n> JS> The unit test changes in this patch remove use of the --show-breaks\n> JS> flags from certain unit tests. The changed --merge-order behaviour\n> JS> changed the annotation that --show-breaks prints for certain test cases.\n> JS> The new behaviour is reasonable and irrelevant to the intent of the tests\n> JS> so that tests have been changed to eliminate the spurious behaviour.\n> \n> If the behaviour of --show-breaks subtly changes, and if that\n> changed behaviour is something still acceptable, why not update\n> the test to show the new expected results since you are updating\n> the test anyway?\n\nI can do it this way, if you prefer. The issue was that the expected output was:\n\n= l2\n| l1 \n| l0\n\nbut became:\n\n^ l2\n| l1\n| l0\n\nThe annotation changes because there is no longer a single head in the\nstart list. There are now multiple heads, it just happens that one of\nthe heads is also a prune point.\n\n> \n> Showing that \"subtle\" change in the diff may draw people's\n> attention and would help you to verify that the behaviour change\n> is not something that would be unacceptable to them.\n\nFair enough, I'll resubmit with a less drastic change to the test case.\n\n> \n> Also if you are changing t6001, could you also merge Mark\n> Allen's BSD portability fix while you are at it?\n> \n> Message-ID: <20050628014337.18986.qmail@web41205.mail.yahoo.com>\n> \n> \n\nOk.\n\njon.\n"},{"id":"5437","messageId":"2cfc4032050629194744e3edbc@mail.gmail.com","threadId":"1050","inReplyTo":"2cfc403205062918336a55e8da@mail.gmail.com","subject":"Re: [PATCH 2/2] Fix for git-rev-list --merge-order B ^A (A,B share common base)","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2005-06-30T02:47:44Z","receivedAt":"2005-06-30T02:47:44Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"> > Also if you are changing t6001, could you also merge Mark\n> > Allen's BSD portability fix while you are at it?\n> >\n> > Message-ID: <20050628014337.18986.qmail@web41205.mail.yahoo.com>\n> >\n> >\n> \n> Ok.\n> \n\nSorry, forgot to do this. However, Mark's patch should still apply as\nit is (or will do so with minimal edits), so I'll leave that to Linus'\ndiscretion.\n\njon.\n-- \nhomepage: http://www.zeta.org.au/~jon/\nblog: http://orwelliantremors.blogspot.com/\n"}]}