| Message ID | 20200619093210.31289-1-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] tests: do not use "slave branch" nomenclature | expand |
On 2020-06-19 11:32:10+0200, Paolo Bonzini <pbonzini@redhat.com> wrote: > Git branches have been qualified as topic branches, integration branches, > development branches, feature branches, release branches and so on. > Git has a branch that is the master *for* development, but it is not > the master *of* any "slave branch": Git does not have slave branches, > and has never had, except for a single testcase that claims otherwise. :) reading this text and the change may give the impression that this is used for feature branch only. I think common terminology in Git's test is this kind of branch is side. In this inaccurate comparison: git grep -E '(branch|checkout|switch).* side ' git grep -E '(branch|checkout|switch).* feature' The former yields more result than the latter. The latter shows only t1090 and t3420. If I were writing this patch, I would go with the former. <xmqqr1umg8fp.fsf@gitster.c.googlers.com> seems to prefer side, too. Other than that, the patch looks good to me. > > Independent of any future change to the naming of the "master" branch, > removing this sole appearance of the term is a strict improvement: it > avoids divisive language, and talking about "feature branch" clarifies > which developer workflow the test is trying to emulate. > > Reported-by: Till Maas <tmaas@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > t/t4014-format-patch.sh | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index 575e079cc2..958c2da56e 100755 > --- a/t/t4014-format-patch.sh > +++ b/t/t4014-format-patch.sh > @@ -81,16 +81,16 @@ test_expect_success 'format-patch --ignore-if-in-upstream handles tags' ' > ' > > test_expect_success "format-patch doesn't consider merge commits" ' > - git checkout -b slave master && > + git checkout -b feature master && > echo "Another line" >>file && > test_tick && > - git commit -am "Slave change #1" && > + git commit -am "Feature branch change #1" && > echo "Yet another line" >>file && > test_tick && > - git commit -am "Slave change #2" && > + git commit -am "Feature branch change #2" && > git checkout -b merger master && > test_tick && > - git merge --no-ff slave && > + git merge --no-ff feature && > git format-patch -3 --stdout >patch && > grep "^From " patch >from && > test_line_count = 3 from > -- > 2.25.4 >
On 19-06-2020 15:02, Paolo Bonzini wrote: > Git branches have been qualified as topic branches, integration branches, > development branches, feature branches, release branches and so on. > Git has a branch that is the master *for* development, but it is not > the master *of* any "slave branch": Git does not have slave branches, > and has never had, except for a single testcase that claims otherwise. :) > I wonder if "claims" is too strong a word here. "... hints otherwise" sounds better to me. > Independent of any future change to the naming of the "master" branch, > removing this sole appearance of the term is a strict improvement: it > avoids divisive language, and talking about "feature branch" clarifies > which developer workflow the test is trying to emulate. > > Reported-by: Till Maas <tmaas@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Other than that and the comment by Danh elsewhere this patch looks good to me. > --- > t/t4014-format-patch.sh | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index 575e079cc2..958c2da56e 100755 > --- a/t/t4014-format-patch.sh > +++ b/t/t4014-format-patch.sh > @@ -81,16 +81,16 @@ test_expect_success 'format-patch --ignore-if-in-upstream handles tags' ' > ' > > test_expect_success "format-patch doesn't consider merge commits" ' > - git checkout -b slave master && > + git checkout -b feature master && > echo "Another line" >>file && > test_tick && > - git commit -am "Slave change #1" && > + git commit -am "Feature branch change #1" && > echo "Yet another line" >>file && > test_tick && > - git commit -am "Slave change #2" && > + git commit -am "Feature branch change #2" && > git checkout -b merger master && > test_tick && > - git merge --no-ff slave && > + git merge --no-ff feature && > git format-patch -3 --stdout >patch && > grep "^From " patch >from && > test_line_count = 3 from >
On 19/06/20 15:00, Đoàn Trần Công Danh wrote: > I think common terminology in Git's test is this kind of branch is side. > In this inaccurate comparison: > > git grep -E '(branch|checkout|switch).* side ' > git grep -E '(branch|checkout|switch).* feature' Side branch is the name that git uses for "parents other than the first one in a merge commit", for example Verify that the tip commit of the side branch being merged is signed with a valid key Feature branch is what you call branches in a workflow that does feature development in a dedicated branch instead of the master branch. In addition to the two that you point out, there are other occurrences of "feature branch". For example in t5520-push.sh: # add a feature branch, keep-merge, that is merged into master, so the # test can try preserving the merge commit (or not) with various # --rebase flags/pull.rebase settings. and that has some resemblance with the format-patch test. (However, t5520-push.sh doesn't call its branch "feature" So I think both terms are acceptable. Certainly "feature branch" is used a lot by git users (and was suggested in the v1 review) even though it's not as prevalent in the source code. Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > Git branches have been qualified as topic branches, integration branches, > development branches, feature branches, release branches and so on. > Git has a branch that is the master *for* development, but it is not > the master *of* any "slave branch": Git does not have slave branches, > and has never had, except for a single testcase that claims otherwise. :) Somebody mentioned "claims" was too strong, but I think the smiley strikes a good balance there. > Independent of any future change to the naming of the "master" branch, > removing this sole appearance of the term is a strict improvement: it > avoids divisive language, and talking about "feature branch" clarifies > which developer workflow the test is trying to emulate. Exactly. As somebody else said, we often call such a branch "side" in the tests, with the (hopefully widely-held) assumption that any development, either new feature or bugfix, would be done on a side branch and then merged to the integration branch. What the test tries to do applies equally to the developer workflow to use a side branch to work on a non feature (like bugfixes), too, but what is written in this patch is good enough, I would say. Thank you to all for commenting. Will queue. > > Reported-by: Till Maas <tmaas@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > t/t4014-format-patch.sh | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index 575e079cc2..958c2da56e 100755 > --- a/t/t4014-format-patch.sh > +++ b/t/t4014-format-patch.sh > @@ -81,16 +81,16 @@ test_expect_success 'format-patch --ignore-if-in-upstream handles tags' ' > ' > > test_expect_success "format-patch doesn't consider merge commits" ' > - git checkout -b slave master && > + git checkout -b feature master && > echo "Another line" >>file && > test_tick && > - git commit -am "Slave change #1" && > + git commit -am "Feature branch change #1" && > echo "Yet another line" >>file && > test_tick && > - git commit -am "Slave change #2" && > + git commit -am "Feature branch change #2" && > git checkout -b merger master && > test_tick && > - git merge --no-ff slave && > + git merge --no-ff feature && > git format-patch -3 --stdout >patch && > grep "^From " patch >from && > test_line_count = 3 from
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 575e079cc2..958c2da56e 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -81,16 +81,16 @@ test_expect_success 'format-patch --ignore-if-in-upstream handles tags' ' ' test_expect_success "format-patch doesn't consider merge commits" ' - git checkout -b slave master && + git checkout -b feature master && echo "Another line" >>file && test_tick && - git commit -am "Slave change #1" && + git commit -am "Feature branch change #1" && echo "Yet another line" >>file && test_tick && - git commit -am "Slave change #2" && + git commit -am "Feature branch change #2" && git checkout -b merger master && test_tick && - git merge --no-ff slave && + git merge --no-ff feature && git format-patch -3 --stdout >patch && grep "^From " patch >from && test_line_count = 3 from
Git branches have been qualified as topic branches, integration branches, development branches, feature branches, release branches and so on. Git has a branch that is the master *for* development, but it is not the master *of* any "slave branch": Git does not have slave branches, and has never had, except for a single testcase that claims otherwise. :) Independent of any future change to the naming of the "master" branch, removing this sole appearance of the term is a strict improvement: it avoids divisive language, and talking about "feature branch" clarifies which developer workflow the test is trying to emulate. Reported-by: Till Maas <tmaas@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- t/t4014-format-patch.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)