From: Phillip Wood Date: Wed, 17 Sep 2025 14:53:34 GMT Subject: Re: [PATCH v3 0/4] breaking changes: switch default initial branch name to "main" Message-ID: <70fa7537-3b89-4c3b-9dea-5a7ea0174a9d@gmail.com> In-Reply-To: On 17/09/2025 10:22, Junio C Hamano wrote: > Junio C Hamano writes: > > We do this without telling them how to permanently configure the > default name of the initial branch, and that design choice is very > much deliberate. That makes sense > It also needs to be noted thta the advise token to squelch the > message is the same advice.defaultBranchName as before, which is > also very much deliberate. The users who do have that configured > are those who _have_ been using Git since before 3.0, and they are > not the target audience for the new advice message. Reusing the > same advise token ensures that they do not have to turn the message > off. That's good Thanks for following up on this. The code changes look good modulo the typo pointed out by Kristoffer. I've left a few thoughts on the test changes below. > diff --git c/t/t0000-basic.sh w/t/t0000-basic.sh > index 2b63e1c86c..54ed80ebd4 100755 > --- c/t/t0000-basic.sh > +++ w/t/t0000-basic.sh > @@ -220,7 +220,8 @@ test_expect_success 'subtest: --verbose option' ' > test_done > EOF > mv t1234-verbose/err t1234-verbose/err+ && > - grep -v "^Initialized empty" t1234-verbose/err+ >t1234-verbose/err && > + grep -v -e "^Initialized empty" -e "^hint:" t1234-verbose/err+ \ > + >t1234-verbose/err && > check_sub_test_lib_test_err t1234-verbose \ > <<-\EOF_OUT 3<<-\EOF_ERR > > ok 1 - passing test > diff --git c/t/t0001-init.sh w/t/t0001-init.sh > index df0040b9ac..5fe7b8c674 100755 > --- c/t/t0001-init.sh > +++ w/t/t0001-init.sh > @@ -562,9 +562,9 @@ test_expect_success 'init warns about invalid init.defaultObjectFormat' ' > test_when_finished "rm -rf repo" && > test_config_global init.defaultObjectFormat garbage && > > - echo "warning: unknown hash algorithm ${SQ}garbage${SQ}" >expect && > + expect="warning: unknown hash algorithm ${SQ}garbage${SQ}" && > git init repo 2>err && > - test_cmp expect err && > + test_grep "$expect" err && > > git -C repo rev-parse --show-object-format >actual && > echo $GIT_DEFAULT_HASH >expected && > @@ -649,9 +649,9 @@ test_expect_success 'init warns about invalid init.defaultRefFormat' ' > test_when_finished "rm -rf repo" && > test_config_global init.defaultRefFormat garbage && > > - echo "warning: unknown ref storage format ${SQ}garbage${SQ}" >expect && > + expect="warning: unknown ref storage format ${SQ}garbage${SQ}" && > git init repo 2>err && > - test_cmp expect err && > + test_grep "$expect" err && The test changes up to this point strike me as somewhat unfortunate as these tests are perfectly content with the existing advice. I assume this stems from us not setting GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME anymore when WITH_BREAKING_CHANGES is enabled. I think we should do something like diff --git b/t/test-lib.sh b/t/test-lib.sh --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -127,13 +127,15 @@ then export GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS fi +# Explicitly set the default branch name for testing, to avoid the +# transitory "git init" warning under --verbose. if test -z "$WITH_BREAKING_CHANGES" then - # Explicitly set the default branch name for testing, to avoid the - # transitory "git init" warning under --verbose. : ${GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME:=master} - export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +else + : ${GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME:=main} fi +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME ################################################################ # It appears that people try to run tests without building... To address that which will also stop the advice appearing when the tests are run with --verbose as well. > git -C repo rev-parse --show-ref-format >actual && > echo $GIT_DEFAULT_REF_FORMAT >expected && > @@ -868,18 +868,18 @@ test_expect_success 'overridden default initial branch name (config)' ' > grep nmb actual > ' > > -test_expect_success !WITH_BREAKING_CHANGES 'advice on unconfigured init.defaultBranch' ' > +test_expect_success 'advice on unconfigured init.defaultBranch' ' > GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= git -c color.advice=always \ > init unconfigured-default-branch-name 2>err && > test_decode_color decoded && > test_grep "hint: " decoded > ' > > -test_expect_success 'advice on unconfigured init.defaultBranch disabled' ' > +test_expect_success 'advice on unconfigured init.defaultBranch can be disabled' ' > test_when_finished "rm -rf no-advice" && > > GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \ > - git -c advice.defaultBranchName=false init no-advice 2>err && > + git -c advice.defaultBranchName=false init no-advice 2>err && The indentation is changed here but not above. I'd be happy to leave this alone, but if we're going to remove the indentation here shouldn't we do the same above? > test_grep ! "hint: " err Do we want to check the advice that is printed now that we have two different messages? Thanks Phillip