Re: [PATCH v3 5/8] init: allow setting the default for the initial branch name via the config

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 24, 2020, 12:56 UTC
Message-ID: <nycvar.QRO.7.76.6.2006241455210.54@tvgsbejvaqbjf.bet>
In-Reply-To: <xmqqftal9su2.fsf@gitster.c.googlers.com>

Hi Junio,

On Tue, 23 Jun 2020, Junio C Hamano wrote:

Show 31 quoted lines

> "Don Goodman-Wilson via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > diff --git a/builtin/init-db.c b/builtin/init-db.c
> > index a898153901..b8634b5f35 100644
> > --- a/builtin/init-db.c
> > +++ b/builtin/init-db.c
> > @@ -269,7 +269,7 @@ static int create_default_files(const char *template_path,
> >  		char *ref;
> >
> >  		if (!initial_branch)
> > -			initial_branch = "master";
> > +			initial_branch = git_default_branch_name();
> >
> >  		ref = xstrfmt("refs/heads/%s", initial_branch);
> >  		if (check_refname_format(ref, 0) < 0)
>
> Continuing with the division of labor between this helper and its
> caller, I had this funny dislike of falling back here and not in the
> caller.  But with the same idea of using "reinit", we could get rid
> of this "if the caller didn't give us initial_branch, fall back
> to..." logic from the function.  The caller may do
>
> 	reinit = create_default_files(...
> 			initial_branch ? initial_branch : "master",
> 			...);
> 	if (reinit || initial_branch)
> 		warning(_(...));
>
> in the previous step and then we can teach the caller to use the
> configured value instead of the hardcoded "master".

While that is really tempting, there is another called of `init_db()` (which calls `create_default_files()`): `builtin/clone.c`. And I do not wish to duplicate the logic there.

So I left this as-is.

Ciao, Dscho

Show 7 quoted lines

>
> That's much better ;-)
>
> Other than that, looks good to me.
>
> Thanks.
>