{"thread":{"id":"53713","subject":"[PATCH v2] tests: do not use \"slave branch\" nomenclature","startedAt":"2020-06-19T09:33:08Z","lastAt":"2020-06-19T17:18:42Z","messageCount":5,"participants":["Paolo Bonzini","Đoàn Trần Công Danh","Kaartic Sivaraam","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"400106","messageId":"20200619093210.31289-1-pbonzini@redhat.com","threadId":"53713","inReplyTo":null,"subject":"[PATCH v2] tests: do not use \"slave branch\" nomenclature","fromName":"Paolo Bonzini","fromEmail":"pbonzini@redhat.com","sentAt":"2020-06-19T09:32:10Z","receivedAt":"2020-06-19T09:33:08Z","isPatch":true,"sender":{"key":"pbonzini@redhat.com","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"Git branches have been qualified as topic branches, integration branches,\ndevelopment branches, feature branches, release branches and so on.\nGit has a branch that is the master *for* development, but it is not\nthe master *of* any \"slave branch\": Git does not have slave branches,\nand has never had, except for a single testcase that claims otherwise. :)\n\nIndependent of any future change to the naming of the \"master\" branch,\nremoving this sole appearance of the term is a strict improvement: it\navoids divisive language, and talking about \"feature branch\" clarifies\nwhich developer workflow the test is trying to emulate.\n\nReported-by: Till Maas <tmaas@redhat.com>\nSigned-off-by: Paolo Bonzini <pbonzini@redhat.com>\n---\n t/t4014-format-patch.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 575e079cc2..958c2da56e 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -81,16 +81,16 @@ test_expect_success 'format-patch --ignore-if-in-upstream handles tags' '\n '\n \n test_expect_success \"format-patch doesn't consider merge commits\" '\n-\tgit checkout -b slave master &&\n+\tgit checkout -b feature master &&\n \techo \"Another line\" >>file &&\n \ttest_tick &&\n-\tgit commit -am \"Slave change #1\" &&\n+\tgit commit -am \"Feature branch change #1\" &&\n \techo \"Yet another line\" >>file &&\n \ttest_tick &&\n-\tgit commit -am \"Slave change #2\" &&\n+\tgit commit -am \"Feature branch change #2\" &&\n \tgit checkout -b merger master &&\n \ttest_tick &&\n-\tgit merge --no-ff slave &&\n+\tgit merge --no-ff feature &&\n \tgit format-patch -3 --stdout >patch &&\n \tgrep \"^From \" patch >from &&\n \ttest_line_count = 3 from\n-- \n2.25.4\n\n"},{"id":"400111","messageId":"20200619130058.GA5027@danh.dev","threadId":"53713","inReplyTo":"20200619093210.31289-1-pbonzini@redhat.com","subject":"Re: [PATCH v2] tests: do not use \"slave branch\" nomenclature","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-06-19T13:00:58Z","receivedAt":"2020-06-19T13:01:05Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-06-19 11:32:10+0200, Paolo Bonzini <pbonzini@redhat.com> wrote:\n> Git branches have been qualified as topic branches, integration branches,\n> development branches, feature branches, release branches and so on.\n> Git has a branch that is the master *for* development, but it is not\n> the master *of* any \"slave branch\": Git does not have slave branches,\n> and has never had, except for a single testcase that claims otherwise. :)\n\nreading this text and the change may give the impression that this is\nused for feature branch only.\n\nI think common terminology in Git's test is this kind of branch is side.\nIn this inaccurate comparison:\n\n\tgit grep -E '(branch|checkout|switch).* side '\n\tgit grep -E '(branch|checkout|switch).* feature'\n\nThe former yields more result than the latter.\nThe latter shows only t1090 and t3420.\n\nIf I were writing this patch, I would go with the former.\n\n<xmqqr1umg8fp.fsf@gitster.c.googlers.com> seems to prefer side, too.\n\nOther than that, the patch looks good to me.\n\n> \n> Independent of any future change to the naming of the \"master\" branch,\n> removing this sole appearance of the term is a strict improvement: it\n> avoids divisive language, and talking about \"feature branch\" clarifies\n> which developer workflow the test is trying to emulate.\n> \n> Reported-by: Till Maas <tmaas@redhat.com>\n> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>\n> ---\n>  t/t4014-format-patch.sh | 8 ++++----\n>  1 file changed, 4 insertions(+), 4 deletions(-)\n> \n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index 575e079cc2..958c2da56e 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -81,16 +81,16 @@ test_expect_success 'format-patch --ignore-if-in-upstream handles tags' '\n>  '\n>  \n>  test_expect_success \"format-patch doesn't consider merge commits\" '\n> -\tgit checkout -b slave master &&\n> +\tgit checkout -b feature master &&\n>  \techo \"Another line\" >>file &&\n>  \ttest_tick &&\n> -\tgit commit -am \"Slave change #1\" &&\n> +\tgit commit -am \"Feature branch change #1\" &&\n>  \techo \"Yet another line\" >>file &&\n>  \ttest_tick &&\n> -\tgit commit -am \"Slave change #2\" &&\n> +\tgit commit -am \"Feature branch change #2\" &&\n>  \tgit checkout -b merger master &&\n>  \ttest_tick &&\n> -\tgit merge --no-ff slave &&\n> +\tgit merge --no-ff feature &&\n>  \tgit format-patch -3 --stdout >patch &&\n>  \tgrep \"^From \" patch >from &&\n>  \ttest_line_count = 3 from\n> -- \n> 2.25.4\n> \n\n-- \nDanh\n"},{"id":"400117","messageId":"8f2bf041-1a04-55cb-05fd-a3802fbfb09d@gmail.com","threadId":"53713","inReplyTo":"20200619093210.31289-1-pbonzini@redhat.com","subject":"Re: [PATCH v2] tests: do not use \"slave branch\" nomenclature","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2020-06-19T13:27:54Z","receivedAt":"2020-06-19T13:29:03Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On 19-06-2020 15:02, Paolo Bonzini wrote:\n> Git branches have been qualified as topic branches, integration branches,\n> development branches, feature branches, release branches and so on.\n> Git has a branch that is the master *for* development, but it is not\n> the master *of* any \"slave branch\": Git does not have slave branches,\n> and has never had, except for a single testcase that claims otherwise. :)\n>\n\nI wonder if \"claims\" is too strong a word here. \"... hints otherwise\"\nsounds better to me.\n\n> Independent of any future change to the naming of the \"master\" branch,\n> removing this sole appearance of the term is a strict improvement: it\n> avoids divisive language, and talking about \"feature branch\" clarifies\n> which developer workflow the test is trying to emulate.\n> \n> Reported-by: Till Maas <tmaas@redhat.com>\n> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>\n\nOther than that and the comment by Danh elsewhere this patch looks\ngood to me.\n\n> ---\n>  t/t4014-format-patch.sh | 8 ++++----\n>  1 file changed, 4 insertions(+), 4 deletions(-)\n> \n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index 575e079cc2..958c2da56e 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -81,16 +81,16 @@ test_expect_success 'format-patch --ignore-if-in-upstream handles tags' '\n>  '\n>  \n>  test_expect_success \"format-patch doesn't consider merge commits\" '\n> -\tgit checkout -b slave master &&\n> +\tgit checkout -b feature master &&\n>  \techo \"Another line\" >>file &&\n>  \ttest_tick &&\n> -\tgit commit -am \"Slave change #1\" &&\n> +\tgit commit -am \"Feature branch change #1\" &&\n>  \techo \"Yet another line\" >>file &&\n>  \ttest_tick &&\n> -\tgit commit -am \"Slave change #2\" &&\n> +\tgit commit -am \"Feature branch change #2\" &&\n>  \tgit checkout -b merger master &&\n>  \ttest_tick &&\n> -\tgit merge --no-ff slave &&\n> +\tgit merge --no-ff feature &&\n>  \tgit format-patch -3 --stdout >patch &&\n>  \tgrep \"^From \" patch >from &&\n>  \ttest_line_count = 3 from\n> \n\n-- \nSivaraam\n"},{"id":"400125","messageId":"e7611e0f-3d62-fc92-7f35-5abcc11f2fd8@redhat.com","threadId":"53713","inReplyTo":"20200619130058.GA5027@danh.dev","subject":"Re: [PATCH v2] tests: do not use \"slave branch\" nomenclature","fromName":"Paolo Bonzini","fromEmail":"pbonzini@redhat.com","sentAt":"2020-06-19T14:23:56Z","receivedAt":"2020-06-19T14:24:09Z","isPatch":true,"sender":{"key":"pbonzini@redhat.com","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"On 19/06/20 15:00, Đoàn Trần Công Danh wrote:\n> I think common terminology in Git's test is this kind of branch is side.\n> In this inaccurate comparison:\n> \n> \tgit grep -E '(branch|checkout|switch).* side '\n> \tgit grep -E '(branch|checkout|switch).* feature'\n\nSide branch is the name that git uses for \"parents other than the first\none in a merge commit\", for example\n\n\tVerify that the tip commit of the side branch being merged is\n\tsigned with a valid key\n\nFeature branch is what you call branches in a workflow that does feature\ndevelopment in a dedicated branch instead of the master branch.  In\naddition to the two that you point out, there are other occurrences of\n\"feature branch\".  For example in t5520-push.sh:\n\n# add a feature branch, keep-merge, that is merged into master, so the\n# test can try preserving the merge commit (or not) with various\n# --rebase flags/pull.rebase settings.\n\nand that has some resemblance with the format-patch test.  (However,\nt5520-push.sh doesn't call its branch \"feature\"\n\nSo I think both terms are acceptable.  Certainly \"feature branch\" is\nused a lot by git users (and was suggested in the v1 review) even though\nit's not as prevalent in the source code.\n\nPaolo\n\n"},{"id":"400152","messageId":"xmqqimfnht3n.fsf@gitster.c.googlers.com","threadId":"53713","inReplyTo":"20200619093210.31289-1-pbonzini@redhat.com","subject":"Re: [PATCH v2] tests: do not use \"slave branch\" nomenclature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-19T17:18:36Z","receivedAt":"2020-06-19T17:18:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paolo Bonzini <pbonzini@redhat.com> writes:\n\n> Git branches have been qualified as topic branches, integration branches,\n> development branches, feature branches, release branches and so on.\n> Git has a branch that is the master *for* development, but it is not\n> the master *of* any \"slave branch\": Git does not have slave branches,\n> and has never had, except for a single testcase that claims otherwise. :)\n\nSomebody mentioned \"claims\" was too strong, but I think the smiley\nstrikes a good balance there.\n\n> Independent of any future change to the naming of the \"master\" branch,\n> removing this sole appearance of the term is a strict improvement: it\n> avoids divisive language, and talking about \"feature branch\" clarifies\n> which developer workflow the test is trying to emulate.\n\nExactly.  As somebody else said, we often call such a branch \"side\"\nin the tests, with the (hopefully widely-held) assumption that any\ndevelopment, either new feature or bugfix, would be done on a side\nbranch and then merged to the integration branch.  What the test\ntries to do applies equally to the developer workflow to use a side\nbranch to work on a non feature (like bugfixes), too, but what is\nwritten in this patch is good enough, I would say.\n\nThank you to all for commenting.\n\nWill queue.\n\n>\n> Reported-by: Till Maas <tmaas@redhat.com>\n> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>\n> ---\n>  t/t4014-format-patch.sh | 8 ++++----\n>  1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index 575e079cc2..958c2da56e 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -81,16 +81,16 @@ test_expect_success 'format-patch --ignore-if-in-upstream handles tags' '\n>  '\n>  \n>  test_expect_success \"format-patch doesn't consider merge commits\" '\n> -\tgit checkout -b slave master &&\n> +\tgit checkout -b feature master &&\n>  \techo \"Another line\" >>file &&\n>  \ttest_tick &&\n> -\tgit commit -am \"Slave change #1\" &&\n> +\tgit commit -am \"Feature branch change #1\" &&\n>  \techo \"Yet another line\" >>file &&\n>  \ttest_tick &&\n> -\tgit commit -am \"Slave change #2\" &&\n> +\tgit commit -am \"Feature branch change #2\" &&\n>  \tgit checkout -b merger master &&\n>  \ttest_tick &&\n> -\tgit merge --no-ff slave &&\n> +\tgit merge --no-ff feature &&\n>  \tgit format-patch -3 --stdout >patch &&\n>  \tgrep \"^From \" patch >from &&\n>  \ttest_line_count = 3 from\n"}]}