Re: [PATCH v3 0/4] breaking changes: switch default initial branch name to "main"
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 17, 2025, 15:21 UTC
- Message-ID
- <xmqq7bxxf6kk.fsf@gitster.g>
- In-Reply-To
- <70fa7537-3b89-4c3b-9dea-5a7ea0174a9d@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
Show 24 quoted lines
> 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_NAMEThat does make it more in line in spirit with what your "breaking-changes: switch default branch to main" does here. Instead of assuming that we won't have "no default set" hint after 3.0, we do assume the "no default set" hint will be there and squelch for a bit longer.
OK.
Show 27 quoted lines
> 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 <err >decoded && >> test_grep "<YELLOW>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?
QUite honestly, I didn't even notice that the line that begins with "init unconfigured..." was a continuation line. We definitely should fix it to match.
>> test_grep ! "hint: " err > > Do we want to check the advice that is printed now that we have two > different messages?
I am not sure; isn't this checking that there is no advice, not two advice messages?
Ahh, you mean two new tests, one with !WITH_BREAKING_CHANGES and one with WITH_BREAKING_CHANGES, to see default_branch_name_advice[] appears in the advice output? I am not sure if that is needed here. Without [PATCH 5/4] there is no test that checks the exact wording of "... will change to main in Git 3.0" message, either, and I think that is a sensible thing *not* to have a test that insists on the exact phrasing for such things.