From: Johannes Schindelin <Johannes.Schindelin@gmx.de> To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com> Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>, git@vger.kernel.org Subject: Re: [PATCH 00/28] Use main as default branch name Date: Fri, 13 Nov 2020 15:22:27 +0100 (CET) [thread overview] Message-ID: <nycvar.QRO.7.76.6.2011131519170.18437@tvgsbejvaqbjf.bet> (raw) In-Reply-To: <87pn4hfmc4.fsf@evledraar.gmail.com> [-- Attachment #1: Type: text/plain, Size: 6813 bytes --] Hi Ævar, On Fri, 13 Nov 2020, Ævar Arnfjörð Bjarmason wrote: > On Thu, Nov 12 2020, Johannes Schindelin via GitGitGadget wrote: > > > This is the big one. This changes the default of init.defaultBranch to main, > > reflecting what many open source projects already did (which was followed by > > GitHub, Azure Repos and others). > > I don't have an issue with the end goal of s/master/main|$whatever/ as > UX here as noted in previous related review rounds. But I don't think > this series as it stands is anywhere near ready. > > This is 27 patches of s/master/main/ in the tests, followed by making > the change to the default in refs.c without as much as a single > documentation update to inform users of the change. Of course, this is _the one_ patch series mentioned in https://github.com/gitgitgadget/git/pull/655 that does _not_ link back to the meta-PR. So you're right, on its own, these 28 patches do not make sense. And if you split off the 28th patch, the remaining 27 patches make even less sense. The reason why I partitioned the changes this way was that it is a _ton_ of patches to review, and in many ways, these 28 patches are the most important ones to review (I didn't want to just rely on the CI builds passing, I reviewed those patches carefully, multiple times, to make sure that they made sense in the goal to transition to `main`). I will respond to the rest of your concerns later, Dscho > > I don't see the point in doing these sorts of test suite changes, it > just seems like needless churn. But I have not read the whole backlog of > previous iterations of this & related patches, so bear with me. > > Why not instead do what we did when we added protocol v2 to the tests, > e.g. in 8a1b0978ab ("test: request GIT_TEST_PROTOCOL_VERSION=0 when > appropriate", 2019-12-23) we simply set the t5515 test to always run > with protocol v1. If you'd do this: > > diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh > index 70a9d2d8ab..939c8b2f83 100755 > --- a/t/t5515-fetch-merge-logic.sh > +++ b/t/t5515-fetch-merge-logic.sh > @@ -11,6 +11,9 @@ test_description='Merge logic in fetch' > GIT_TEST_PROTOCOL_VERSION=0 > export GIT_TEST_PROTOCOL_VERSION > > +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master > +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME > + > . ./test-lib.sh > > build_script () { > > Of course that also needs: > > index fa347ed3e1..2d700a171b 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -1711,10 +1711,8 @@ test_lazy_prereq SHA1 ' > test_lazy_prereq REBASE_P ' > test -z "$GIT_TEST_SKIP_REBASE_P" > ' > -# Special-purpose prereq for transitioning to a new default branch name: > -# Some tests need more than just a mindless (case-preserving) s/master/main/g > -# replacement. The non-trivial adjustments are guarded behind this > -# prerequisite, acting kind of as a feature flag > -test_lazy_prereq PREPARE_FOR_MAIN_BRANCH ' > - test "$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME" = main > + > +test_lazy_prereq MAIN_BRANCH_IS_MASTER ' > + test -z "$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME" || > + test "$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME" = "master" > ' > > Because (but see "later" a few paragraphs later), the logic as it stands > on "master" is entirely backwards & broken. I.e. if you don't set > GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME or set it to "master" we now skip a > bunch of tests ... which only work if the value is "master". If you set > it to "main" they'll break. > > Then instead of ending up with a hardcoding of "main" we'd be able to > run the tests with a new custom branch name (even one with whitespace in > it). That coverage is plenty to test any branch name hardcoding. > > The remaining tests that would still have "master" would then just be > dealt with by the same thing we did for the too-protocol-v1-specific > tests, i.e.: > > diff --git a/t/t3502-cherry-pick-merge.sh b/t/t3502-cherry-pick-merge.sh > index 8b635a196d..354f9bd851 100755 > --- a/t/t3502-cherry-pick-merge.sh > +++ b/t/t3502-cherry-pick-merge.sh > @@ -8,6 +8,8 @@ test_description='cherry picking and reverting a merge > > ' > > +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master > +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME > . ./test-lib.sh > > test_expect_success setup ' > > Except unlike protocol v1/v2 we're really not gaining anything in > coverage by going further, it's not the point of any of these tests to > test for the default branch name. It's a *lot* of churn, > > "Later": After writing the above I see from 704fed9ea2 ("tests: start > moving to a different default main branch name", 2020-10-23) that this > state of affairs is intentional. But doesn't discuss why the more usual > (because we've done it that way a few times before) v1/v2-esque > transition wasn't done instead. > > Doing it the other way still seems better. We're now not running our > full tests in anticipation for this series landing on "master", and > after applying this we're still not running all of them. > > Anyway, enough with discussing this detail of the test suite churn. My > main objection to this version of it is: > > After this series as it stands lands what's a rather big UI change in > git isn't discussed *anywhere* in the docs. > > I'm not saying we need some similar s/master/main/g in Documentation/ as > what's being done here in t/, but I think a bare minimum for a rather > big UI change like this is: > > 1) Let's at least talk about it in some way in git-init(1), i.e. that > it used to be "master" before version so-and-so and is now > different. With this series applied we still say: > > Documentation/git-init.txt:If not specified, fall back to the default name: `master`. > > 2) Ditto in the init.defaultBranch and things like that > > 3) After all this effort at eliminating "master" entirely in the tests > we're still shipping a sample hook that expects "master" on "git > init", and breaks now that it's "main" > > And maybe stuff like: > > 4) Have "git init" emit some new advice message saying that the default > has changed and we've init-ed a new repo with a "main" branch. > > Our primary concern should be the vast majority of users who don't > follow this topic on Twitter, don't read this mailing list, and are just > going to waste 10 minutes of their day because some script they've had > working for years using "git init" did something unexpected. > > Let's at least aim to make that 5 minutes instead by making the change > more discoverable. >
next prev parent reply other threads:[~2020-11-13 14:23 UTC|newest] Thread overview: 163+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-12 22:43 Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 01/28] t0060: preemptively adjust alignment Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 02/28] t[01]*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 03/28] t2*: " Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 04/28] t3[0-3]*: " Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 05/28] t3416: preemptively adjust alignment in a comment Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 06/28] t34*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 07/28] t3[5-9]*: " Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 08/28] t4*: " Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 09/28] t5323: prepare centered comment for `master` -> `main` Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 10/28] t5[0-4]*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 11/28] t5503: prepare aligned comment for replacing `master` with `main` Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 12/28] t550*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 13/28] t551*: " Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 14/28] t55[23]*: " Johannes Schindelin via GitGitGadget 2020-11-13 10:02 ` Ævar Arnfjörð Bjarmason 2020-11-13 14:18 ` Johannes Schindelin 2020-11-16 20:07 ` Junio C Hamano 2020-11-12 22:43 ` [PATCH 15/28] t55[4-9]*: " Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 16/28] t5[6-9]*: " Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 17/28] t6[0-3]*: " Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 18/28] t64*: preemptively adjust alignment to prepare for `master` -> `main` Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 19/28] t6[4-9]*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 20/28] t7[0-4]*: " Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 21/28] t7[5-9]*: " Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 22/28] t8*: " Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 23/28] t9[0-4]*: " Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 24/28] t9[5-7]*: " Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 25/28] tests(git-p4): transition to the default branch name `main` Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 26/28] t99*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 27/28] tests: drop prereq `PREPARE_FOR_MAIN_BRANCH` where no longer needed Johannes Schindelin via GitGitGadget 2020-11-12 22:43 ` [PATCH 28/28] Change the default branch name to `main` Don Goodman-Wilson via GitGitGadget 2020-11-13 9:57 ` Ævar Arnfjörð Bjarmason 2020-11-13 14:09 ` Johannes Schindelin 2020-11-16 20:04 ` Junio C Hamano 2020-11-13 0:11 ` [PATCH 00/28] Use main as default branch name Felipe Contreras 2020-11-13 12:55 ` Ævar Arnfjörð Bjarmason 2020-11-13 14:22 ` Johannes Schindelin [this message] 2020-11-13 16:13 ` [RFC/PATCH] tests: support testing with an arbitrary default branch (sort of) Ævar Arnfjörð Bjarmason 2020-11-13 19:14 ` Jeff King 2020-11-13 22:00 ` Johannes Schindelin 2020-11-13 22:39 ` Johannes Schindelin 2020-11-13 23:49 ` Ævar Arnfjörð Bjarmason [not found] ` <nycvar.QRO.7.76.6.2011162118060.18437@tvgsbejvaqbjf.bet> 2020-11-18 13:32 ` Ævar Arnfjörð Bjarmason 2020-11-18 14:16 ` Felipe Contreras 2020-11-20 11:36 ` Ævar Arnfjörð Bjarmason 2020-11-20 16:00 ` Felipe Contreras 2020-11-18 15:56 ` Junio C Hamano 2020-11-19 9:32 ` Johannes Schindelin 2020-11-19 19:35 ` Junio C Hamano 2020-11-22 19:07 ` Johannes Schindelin 2020-11-19 10:46 ` Johannes Schindelin 2020-11-14 0:25 ` Felipe Contreras 2020-11-13 17:42 ` [PATCH 00/28] Use main as default branch name Felipe Contreras 2020-11-14 6:13 ` Junio C Hamano 2020-11-16 19:52 ` Junio C Hamano 2020-11-17 16:10 ` Johannes Schindelin 2020-11-17 19:09 ` Junio C Hamano 2020-11-18 20:58 ` Johannes Schindelin 2020-11-17 16:12 ` [PATCH v2 00/27] tests: use " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 01/27] t0060: preemptively adjust alignment Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 02/27] t[01]*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 03/27] t2*: " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 04/27] t3[0-3]*: " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 05/27] t3416: preemptively adjust alignment in a comment Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 06/27] t34*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 07/27] t3[5-9]*: " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 08/27] t4*: " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 09/27] t5323: prepare centered comment for `master` -> `main` Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 10/27] t5[0-4]*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 11/27] t5503: prepare aligned comment for replacing `master` with `main` Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 12/27] t550*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 13/27] t551*: " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 14/27] t55[23]*: " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 15/27] t55[4-9]*: " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 16/27] t5[6-9]*: " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 17/27] t6[0-3]*: " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 18/27] t64*: preemptively adjust alignment to prepare for `master` -> `main` Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 19/27] t6[4-9]*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 20/27] t7[0-4]*: " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 21/27] t7[5-9]*: " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 22/27] t8*: " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 23/27] t9[0-4]*: " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 24/27] t9[5-7]*: " Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 25/27] tests(git-p4): transition to the default branch name `main` Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 26/27] t99*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-17 16:12 ` [PATCH v2 27/27] tests: drop prereq `PREPARE_FOR_MAIN_BRANCH` where no longer needed Johannes Schindelin via GitGitGadget 2020-11-18 11:48 ` [PATCH v2 28/27] tests: run tests omitted by PREPARE_FOR_MAIN_BRANCH Ævar Arnfjörð Bjarmason 2020-11-18 23:56 ` Johannes Schindelin 2020-11-19 19:30 ` Junio C Hamano 2020-11-20 13:09 ` Johannes Schindelin 2020-11-20 18:08 ` Junio C Hamano 2020-11-18 23:44 ` [PATCH v3 00/28] tests: use main as default branch name Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 01/28] tests: mark tests relying on the current default for `init.defaultBranch` Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 02/28] t0060: preemptively adjust alignment Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 03/28] t[01]*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 04/28] t2*: " Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 05/28] t3[0-3]*: " Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 06/28] t3416: preemptively adjust alignment in a comment Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 07/28] t34*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 08/28] t3[5-9]*: " Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 09/28] t4*: " Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 10/28] t5323: prepare centered comment for `master` -> `main` Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 11/28] t5[0-4]*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 12/28] t5503: prepare aligned comment for replacing `master` with `main` Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 13/28] t550*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 14/28] t551*: " Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 15/28] t55[23]*: " Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 16/28] t55[4-9]*: " Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 17/28] t5[6-9]*: " Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 18/28] t6[0-3]*: " Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 19/28] t64*: preemptively adjust alignment to prepare for `master` -> `main` Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 20/28] t6[4-9]*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 21/28] t7[0-4]*: " Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 22/28] t7[5-9]*: " Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 23/28] t8*: " Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 24/28] t9[0-4]*: " Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 25/28] t9[5-7]*: " Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 26/28] tests(git-p4): transition to the default branch name `main` Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 27/28] t99*: adjust the references to the default branch name "main" Johannes Schindelin via GitGitGadget 2020-11-18 23:44 ` [PATCH v3 28/28] tests: drop prereq `PREPARE_FOR_MAIN_BRANCH` where no longer needed Johannes Schindelin via GitGitGadget 2020-11-19 23:35 ` [PATCH v3 00/28] tests: use main as default branch name Junio C Hamano 2020-11-17 20:48 ` [PATCH 00/28] Use " Eric W. Biederman 2020-11-17 22:47 ` Felipe Contreras 2020-11-17 23:33 ` Jeff King 2020-11-18 0:07 ` Junio C Hamano 2020-11-18 1:25 ` Jeff King 2020-11-18 2:40 ` Jonathan Nieder 2020-11-18 5:43 ` Junio C Hamano 2020-11-18 11:32 ` Johannes Schindelin 2020-11-18 15:43 ` Junio C Hamano 2020-11-19 9:22 ` Johannes Schindelin 2020-11-18 0:18 ` Felipe Contreras 2020-11-18 1:22 ` Jeff King 2020-11-18 1:45 ` Felipe Contreras 2020-11-18 2:06 ` Jeff King 2020-11-18 2:39 ` Jonathan Nieder 2020-11-18 10:12 ` Felipe Contreras 2020-11-18 9:04 ` Felipe Contreras 2020-11-17 22:55 ` Junio C Hamano 2020-11-17 23:16 ` Felipe Contreras 2020-11-18 0:10 ` Junio C Hamano 2020-11-18 0:51 ` Felipe Contreras 2020-11-18 23:45 ` Philip Oakley 2020-11-19 0:00 ` Johannes Schindelin 2020-11-19 0:30 ` Philip Oakley 2020-11-19 0:30 ` Peter Hadlaw 2020-11-19 0:44 ` Philip Oakley 2020-11-19 0:55 ` Peter Hadlaw 2020-11-19 1:51 ` Junio C Hamano 2020-11-19 10:35 ` Philip Oakley 2020-11-19 11:10 ` Sergey Organov 2020-11-19 11:50 ` Philip Oakley 2020-11-19 12:33 ` Sergey Organov 2020-11-21 23:01 ` Junio C Hamano 2020-11-22 10:21 ` Sergey Organov 2020-11-22 11:20 ` Philip Oakley 2020-11-22 13:23 ` Sergey Organov 2020-11-22 20:18 ` Philip Oakley 2020-11-22 21:56 ` Felipe Contreras 2020-11-22 23:54 ` Junio C Hamano 2020-11-18 2:56 ` Jonathan Nieder 2020-11-18 5:57 ` Junio C Hamano
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=nycvar.QRO.7.76.6.2011131519170.18437@tvgsbejvaqbjf.bet \ --to=johannes.schindelin@gmx.de \ --cc=avarab@gmail.com \ --cc=git@vger.kernel.org \ --cc=gitgitgadget@gmail.com \ --subject='Re: [PATCH 00/28] Use main as default branch name' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).