Subject: Re: [PATCH 02/16] refs: rename `init_db` callback to avoid confusion

From: Patrick Steinhardt <[email-masked]>
Date: Fri, 17 May 2024 09:08:47 +0200

On Thu, May 16, 2024 at 11:24:36AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <[email-masked]> writes:
> 
> > Reference backends have two callbacks `init` and `init_db`. The
> > similarity of these two callbacks has repeatedly tripped myself whenever
> > I was looking at those, where I always had to look up which of them does
> > what.
> >
> > Rename the `init_db` callback to `create`, which should hopefully be
> > clearer.
> 
> Hmph, create() may be clearer than init_db(), but then I am not sure
> what init() would do, differently from create(), so this rename
> takes me back to the puzzled square one state X-<.
> 
> I am guessing that create is about creating on-disk structure, while
> init is about in-core structure out of an existing on-disk
> structure?  Once I understand the differences in these two things,
> it is much less troublesome to tell them apart, regardless of what
> they are called.  Between .init and .init_db, it would be obvious
> that the latter is about on-disk thing, without a rename done by
> this step.  On the other hand, contrast between <create, init> is
> just as opaque as <init_db, init>---the names do not tell readers
> that these two are about on-disk and in-core structures.
> 
> Just my confused impression.

I certainly wouldn't claim that `create()` vs `init()` is perfect,
either. We could easily avoid that confusion if we were happy to make it
more verbose, e.g. by calling it `create_on_disk()`. I don't really
think that we also need to do the same for `init()`, mostly because it
is a common idiom in our codebase to have `init()` initialize data
structures. But that may also be my own personal bias.

I'll go with `create_on_disk()` for now. It's more verbose, but given
that this function isn't called all that much I think it's fine overall.

Patrick