diff mbox series

[3/5] t3200: avoid variations of the `master` branch name

Message ID 2f1d0a2df41f567bc1a8bc446c26e1cb8b6dc36c.1600279853.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Inclusive naming, part II | expand

Commit Message

Phillip Wood via GitGitGadget Sept. 16, 2020, 6:10 p.m. UTC 
From: Johannes Schindelin <johannes.schindelin@gmx.de>

To avoid branch names with a loaded history, we already started to avoid
using the name "master" in a couple instances.

The `t3200-branch.sh` script uses variations of this name for branches
other than the default one. So let's change those names, as
"lowest-hanging fruits" in the effort to use more inclusive naming
throughout Git's source code.

In this particular instance, this requires a couple of non-trivial
adjustments, as the aligned output depends on the maximum length of the
displayed branches (which we now changed), and also on the alphabetical
order (which we now changed, too).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3200-branch.sh | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

Comments

Jeff King Sept. 16, 2020, 9:11 p.m. UTC | #1 
On Wed, Sep 16, 2020 at 06:10:51PM +0000, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> To avoid branch names with a loaded history, we already started to avoid
> using the name "master" in a couple instances.
> 
> The `t3200-branch.sh` script uses variations of this name for branches
> other than the default one. So let's change those names, as
> "lowest-hanging fruits" in the effort to use more inclusive naming
> throughout Git's source code.

A few of these are kind of odd after only this patch. E.g.:

> -test_expect_success 'git branch -M master2 master2 should work when master is checked out' '
> +test_expect_success 'git branch -M main2 main2 should work when master is checked out' '
>  	git checkout master &&
> -	git branch master2 &&
> -	git branch -M master2 master2
> +	git branch main2 &&
> +	git branch -M main2 main2
>  '

The point of "master2" is that it wasn't "master". But now "main2" is
kind of a weird name, because it has a "2" but isn't related to
anything. If we eventually move the base branch name to "main", they'll
line up again.

I'm on the fence on whether this matters. It's a temporary
inconsistency, assuming we eventually move to "main" as the default. We
_could_ push this change off to that patch, too, but it does make it
more noisy.

But it may be that the connection to "master" here is not all that
important in the first place. And so perhaps an even better patch (both
at this stage and in the long run) is to give it a more descriptive
name. If all of these could just be "branch2", "branch3", etc, then that
alone would be better than "master2", IMHO. I'm not sure if the shared
"ma" prefix matters, though (I know in some tests it does because we're
testing glob matching).

Again, I'm on the fence whether this is exploring too deeply. It's an
opportunity to improve the tests while we're changing them. But at the
same time, I doubt anybody cares too much overall, so it feels a bit
like make-work.

-Peff
Junio C Hamano Sept. 16, 2020, 10:28 p.m. UTC | #2 
Jeff King <peff@peff.net> writes:

> I'm on the fence on whether this matters. It's a temporary
> inconsistency, assuming we eventually move to "main" as the default. We
> _could_ push this change off to that patch, too, but it does make it
> more noisy.

Another way to handle this is perhaps to teach test-lib.sh a way to
tell it that we want to live in the world where the initial default
branch name is 'main' and use that at the beginning of these select
test scripts like t3200.  Then we can do three related things in a
single patch to t3200, which are:

 - Declare that any "git init" in this test (including the initial
   one) uses 'main' as the default branch name;

 - rename 'master' used in the test to 'main'

 - rename 'master2' used in the test to 'main2'

and it would eliminate the awkwardness.

The change to test-lib.sh would likely to use init.defaultBranch
which also would be a good thing.
Jeff King Sept. 16, 2020, 10:39 p.m. UTC | #3 
On Wed, Sep 16, 2020 at 03:28:21PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I'm on the fence on whether this matters. It's a temporary
> > inconsistency, assuming we eventually move to "main" as the default. We
> > _could_ push this change off to that patch, too, but it does make it
> > more noisy.
> 
> Another way to handle this is perhaps to teach test-lib.sh a way to
> tell it that we want to live in the world where the initial default
> branch name is 'main' and use that at the beginning of these select
> test scripts like t3200.  Then we can do three related things in a
> single patch to t3200, which are:
> 
>  - Declare that any "git init" in this test (including the initial
>    one) uses 'main' as the default branch name;
> 
>  - rename 'master' used in the test to 'main'
> 
>  - rename 'master2' used in the test to 'main2'
> 
> and it would eliminate the awkwardness.
> 
> The change to test-lib.sh would likely to use init.defaultBranch
> which also would be a good thing.

Yeah, I'd be perfectly happy with that.

-Peff
Johannes Schindelin Sept. 20, 2020, 3:43 p.m. UTC | #4 
Hi Junio & Peff,

On Wed, 16 Sep 2020, Jeff King wrote:

> On Wed, Sep 16, 2020 at 03:28:21PM -0700, Junio C Hamano wrote:
>
> > Jeff King <peff@peff.net> writes:
> >
> > > I'm on the fence on whether this matters. It's a temporary
> > > inconsistency, assuming we eventually move to "main" as the default.
> > > We _could_ push this change off to that patch, too, but it does make
> > > it more noisy.
> >
> > Another way to handle this is perhaps to teach test-lib.sh a way to
> > tell it that we want to live in the world where the initial default
> > branch name is 'main' and use that at the beginning of these select
> > test scripts like t3200.  Then we can do three related things in a
> > single patch to t3200, which are:
> >
> >  - Declare that any "git init" in this test (including the initial
> >    one) uses 'main' as the default branch name;
> >
> >  - rename 'master' used in the test to 'main'
> >
> >  - rename 'master2' used in the test to 'main2'
> >
> > and it would eliminate the awkwardness.
> >
> > The change to test-lib.sh would likely to use init.defaultBranch
> > which also would be a good thing.
>
> Yeah, I'd be perfectly happy with that.

I do want to introduce something like that in the patch series after the
next one.

However, in this instance, I think it makes more sense to use a separate
name altogether. I settled for using `topic` instead of `main2`, and
`new-topic` instead of `main3` locally.

Ciao,
Dscho
Junio C Hamano Sept. 21, 2020, 9:26 p.m. UTC | #5 
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> > Jeff King <peff@peff.net> writes:
>> >
>> > > I'm on the fence on whether this matters. It's a temporary
>> > > inconsistency, assuming we eventually move to "main" as the default.
>> > > We _could_ push this change off to that patch, too, but it does make
>> > > it more noisy.
>> > ...
> However, in this instance, I think it makes more sense to use a separate
> name altogether. I settled for using `topic` instead of `main2`, and
> `new-topic` instead of `main3` locally.

I think that is sensible.  Configuration does not have to be used as
an escape hatch to make 'main2' less awkward---if we can avoid
'main2' (or 'master2'), that would be sufficient.
diff mbox series

Patch

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 4c0734157b..c1b881c0b8 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -234,10 +234,10 @@  test_expect_success 'git branch -M master master should work when master is chec
 	git branch -M master master
 '
 
-test_expect_success 'git branch -M master2 master2 should work when master is checked out' '
+test_expect_success 'git branch -M main2 main2 should work when master is checked out' '
 	git checkout master &&
-	git branch master2 &&
-	git branch -M master2 master2
+	git branch main2 &&
+	git branch -M main2 main2
 '
 
 test_expect_success 'git branch -v -d t should work' '
@@ -324,8 +324,8 @@  test_expect_success 'git branch --list -v with --abbrev' '
 test_expect_success 'git branch --column' '
 	COLUMNS=81 git branch --column=column >actual &&
 	cat >expect <<\EOF &&
-  a/b/c     bam       foo       l       * master    mb        o/o       q
-  abc       bar       j/k       m/m       master2   n         o/p       r
+  a/b/c    bam      foo      l        main2    mb       o/o      q
+  abc      bar      j/k      m/m    * master   n        o/p      r
 EOF
 	test_cmp expect actual
 '
@@ -345,8 +345,8 @@  test_expect_success 'git branch --column with an extremely long branch name' '
   j/k
   l
   m/m
+  main2
 * master
-  master2
   mb
   n
   o/o
@@ -365,8 +365,8 @@  test_expect_success 'git branch with column.*' '
 	git config --unset column.branch &&
 	git config --unset column.ui &&
 	cat >expect <<\EOF &&
-  a/b/c   bam   foo   l   * master    mb   o/o   q
-  abc     bar   j/k   m/m   master2   n    o/p   r
+  a/b/c   bam   foo   l     main2    mb   o/o   q
+  abc     bar   j/k   m/m * master   n    o/p   r
 EOF
 	test_cmp expect actual
 '
@@ -377,7 +377,7 @@  test_expect_success 'git branch --column -v should fail' '
 
 test_expect_success 'git branch -v with column.ui ignored' '
 	git config column.ui column &&
-	COLUMNS=80 git branch -v | cut -c -10 | sed "s/ *$//" >actual &&
+	COLUMNS=80 git branch -v | cut -c -9 | sed "s/ *$//" >actual &&
 	git config --unset column.ui &&
 	cat >expect <<\EOF &&
   a/b/c
@@ -388,8 +388,8 @@  test_expect_success 'git branch -v with column.ui ignored' '
   j/k
   l
   m/m
+  main2
 * master
-  master2
   mb
   n
   o/o
@@ -597,10 +597,10 @@  test_expect_success 'git branch -C master master should work when master is chec
 	git branch -C master master
 '
 
-test_expect_success 'git branch -C master5 master5 should work when master is checked out' '
+test_expect_success 'git branch -C main5 main5 should work when master is checked out' '
 	git checkout master &&
-	git branch master5 &&
-	git branch -C master5 master5
+	git branch main5 &&
+	git branch -C main5 main5
 '
 
 test_expect_success 'git branch -C ab cd should overwrite existing config for cd' '
@@ -711,11 +711,11 @@  test_expect_success 'deleting a self-referential symref' '
 '
 
 test_expect_success 'renaming a symref is not allowed' '
-	git symbolic-ref refs/heads/master2 refs/heads/master &&
-	test_must_fail git branch -m master2 master3 &&
-	git symbolic-ref refs/heads/master2 &&
+	git symbolic-ref refs/heads/main2 refs/heads/master &&
+	test_must_fail git branch -m main2 main3 &&
+	git symbolic-ref refs/heads/main2 &&
 	test_path_is_file .git/refs/heads/master &&
-	test_path_is_missing .git/refs/heads/master3
+	test_path_is_missing .git/refs/heads/main3
 '
 
 test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' '