{"thread":{"id":"53976","subject":"[PATCH] Revert \"contrib: subtree: adjust test to change in fmt-merge-msg\"","startedAt":"2020-08-03T18:57:55Z","lastAt":"2020-08-04T13:12:32Z","messageCount":4,"participants":["Emily Shaffer","Junio C Hamano","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"402736","messageId":"20200803185749.1441344-1-emilyshaffer@google.com","threadId":"53976","inReplyTo":null,"subject":"[PATCH] Revert \"contrib: subtree: adjust test to change in fmt-merge-msg\"","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-08-03T18:57:49Z","receivedAt":"2020-08-03T18:57:55Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"This reverts commit 508fd8e8baf3e18ee40b2cf0b8899188a8506d07.\n\nIn 6e6029a8 (fmt-merge-msg: allow merge destination to be omitted again)\nwe get back the behavior where merges against 'master', by default, do\nnot include \"into 'master'\" at the end of the merge message. This test\nfix is no longer needed.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\n\nI don't think anybody else sent this yet. Because the default value of\nmerge.suppressDest suppresses 'master', the commit messages are wrong\nagain for these tests. Noticed during a release for $DAYJOB. Thanks -\nlooks like Linus was right that we get to remove lots more instances of\n'master' from codebase with that config patch.\n\n - Emily\n\n contrib/subtree/t/t7900-subtree.sh | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh\nindex 53d7accf94..57ff4b25c1 100755\n--- a/contrib/subtree/t/t7900-subtree.sh\n+++ b/contrib/subtree/t/t7900-subtree.sh\n@@ -196,8 +196,7 @@ test_expect_success 'merge new subproj history into sub dir/ with --prefix' '\n \t\tcd \"$subtree_test_count\" &&\n \t\tgit fetch ./\"sub proj\" master &&\n \t\tgit subtree merge --prefix=\"sub dir\" FETCH_HEAD &&\n-\t\tcheck_equal \"$(last_commit_message)\" \\\n-\t\t\t\"Merge commit '\\''$(git rev-parse FETCH_HEAD)'\\'' into master\"\n+\t\tcheck_equal \"$(last_commit_message)\" \"Merge commit '\\''$(git rev-parse FETCH_HEAD)'\\''\"\n \t)\n '\n \n@@ -274,8 +273,7 @@ test_expect_success 'merge new subproj history into subdir/ with a slash appende\n \t\tcd \"$test_count\" &&\n \t\tgit fetch ./subproj master &&\n \t\tgit subtree merge --prefix=subdir/ FETCH_HEAD &&\n-\t\tcheck_equal \"$(last_commit_message)\" \\\n-\t\t\t\"Merge commit '\\''$(git rev-parse FETCH_HEAD)'\\'' into master\"\n+\t\tcheck_equal \"$(last_commit_message)\" \"Merge commit '\\''$(git rev-parse FETCH_HEAD)'\\''\"\n \t)\n '\n \n-- \n2.28.0.163.g6104cc2f0b6-goog\n\n"},{"id":"402745","messageId":"xmqqlfivv7ky.fsf@gitster.c.googlers.com","threadId":"53976","inReplyTo":"20200803185749.1441344-1-emilyshaffer@google.com","subject":"Re: [PATCH] Revert \"contrib: subtree: adjust test to change in fmt-merge-msg\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-03T19:43:25Z","receivedAt":"2020-08-03T19:43:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer <emilyshaffer@google.com> writes:\n\n> This reverts commit 508fd8e8baf3e18ee40b2cf0b8899188a8506d07.\n>\n> In 6e6029a8 (fmt-merge-msg: allow merge destination to be omitted again)\n> we get back the behavior where merges against 'master', by default, do\n> not include \"into 'master'\" at the end of the merge message. This test\n> fix is no longer needed.\n>\n> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>\n> ---\n>\n> I don't think anybody else sent this yet. Because the default value of\n> merge.suppressDest suppresses 'master', the commit messages are wrong\n> again for these tests. Noticed during a release for $DAYJOB. Thanks -\n> looks like Linus was right that we get to remove lots more instances of\n> 'master' from codebase with that config patch.\n\nI as many others personally do not care about subtree and normally\ndo not run its tests, so it is understandable that this was missed.\nThanks for catching.\n\nI wonder if it makes more sense to move this (or other tests in\ncontrib/ if any) in the normal test suite so that we'd notice its\nbreakage, though.\n\n>  contrib/subtree/t/t7900-subtree.sh | 6 ++----\n>  1 file changed, 2 insertions(+), 4 deletions(-)\n>\n> diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh\n> index 53d7accf94..57ff4b25c1 100755\n> --- a/contrib/subtree/t/t7900-subtree.sh\n> +++ b/contrib/subtree/t/t7900-subtree.sh\n> @@ -196,8 +196,7 @@ test_expect_success 'merge new subproj history into sub dir/ with --prefix' '\n>  \t\tcd \"$subtree_test_count\" &&\n>  \t\tgit fetch ./\"sub proj\" master &&\n>  \t\tgit subtree merge --prefix=\"sub dir\" FETCH_HEAD &&\n> -\t\tcheck_equal \"$(last_commit_message)\" \\\n> -\t\t\t\"Merge commit '\\''$(git rev-parse FETCH_HEAD)'\\'' into master\"\n> +\t\tcheck_equal \"$(last_commit_message)\" \"Merge commit '\\''$(git rev-parse FETCH_HEAD)'\\''\"\n>  \t)\n>  '\n>  \n> @@ -274,8 +273,7 @@ test_expect_success 'merge new subproj history into subdir/ with a slash appende\n>  \t\tcd \"$test_count\" &&\n>  \t\tgit fetch ./subproj master &&\n>  \t\tgit subtree merge --prefix=subdir/ FETCH_HEAD &&\n> -\t\tcheck_equal \"$(last_commit_message)\" \\\n> -\t\t\t\"Merge commit '\\''$(git rev-parse FETCH_HEAD)'\\'' into master\"\n> +\t\tcheck_equal \"$(last_commit_message)\" \"Merge commit '\\''$(git rev-parse FETCH_HEAD)'\\''\"\n>  \t)\n>  '\n"},{"id":"402762","messageId":"20200803223945.GF2965447@google.com","threadId":"53976","inReplyTo":"xmqqlfivv7ky.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] Revert \"contrib: subtree: adjust test to change in fmt-merge-msg\"","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-08-03T22:39:45Z","receivedAt":"2020-08-03T22:39:53Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Mon, Aug 03, 2020 at 12:43:25PM -0700, Junio C Hamano wrote:\n> \n> Emily Shaffer <emilyshaffer@google.com> writes:\n> I wonder if it makes more sense to move this (or other tests in\n> contrib/ if any) in the normal test suite so that we'd notice its\n> breakage, though.\n\nI'd welcome that change. We have some scripted step during our release\nprocess to run those specially, and it'd be nice not to have to.\n\n - Emily\n"},{"id":"402812","messageId":"5cb1c0f8-53c4-f7c5-d1aa-a9241c511fd0@gmail.com","threadId":"53976","inReplyTo":"20200803223945.GF2965447@google.com","subject":"Re: [PATCH] Revert \"contrib: subtree: adjust test to change in fmt-merge-msg\"","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-08-04T13:12:28Z","receivedAt":"2020-08-04T13:12:32Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/3/2020 6:39 PM, Emily Shaffer wrote:\n> On Mon, Aug 03, 2020 at 12:43:25PM -0700, Junio C Hamano wrote:\n>>\n>> Emily Shaffer <emilyshaffer@google.com> writes:\n>> I wonder if it makes more sense to move this (or other tests in\n>> contrib/ if any) in the normal test suite so that we'd notice its\n>> breakage, though.\n> \n> I'd welcome that change. We have some scripted step during our release\n> process to run those specially, and it'd be nice not to have to.\n\nI support that addition of contrib/ tests in the normal test suite,\nor even just part of the CI builds. Anything to help catch these things\nearlier.\n\nThanks,\n-Stolee\n"}]}