Re: [PATCH v13 13/13] Add some reftable testing infrastructure

Han-Wen Nienhuys [off-list ref] writes:

On Wed, May 13, 2020 at 9:57 PM Junio C Hamano [off-list ref] wrote:

quoted

quoted

--- a/refs.c
+++ b/refs.c

@@ -1742,7 +1742,7 @@ struct ref_store *get_main_ref_store(struct repository *r)
      r->refs_private = ref_store_init(r->gitdir,
                                       r->ref_storage_format ?
                                               r->ref_storage_format :
-                                              DEFAULT_REF_STORAGE,
+                                              default_ref_storage(),
                                       REF_STORE_ALL_CAPS);
      return r->refs_private;
 }

Would it make sense to let NULL stand for "we use the default,
whatever it is" so that anybody outside the implementation of the
refs API does not have to care what the default is, and make
ref_store_init() function the only thing that knows what it is?

Is this your strong recommendation in the form of a question? :)

If it is just a question: maybe. I don't know enough of Git to say for sure.

I try not to ask a rhetorical question without marking it explicitly
as such.  The question "instead of having to sprinkle ?: all over
the place on the callers' side, would it make it better or worse to
give a 'not decided' default to the ref_storage_format field?" came
to mind while reading the series.

How do submodules decide on other settings, such as the hash ID?

I am not sure the issue of "what default to use" is limited to the
submodule case (e.g. "git init" and "git clone" starts from having
no "current repository" and they need to make the decision among
available choices).  As I've seen this series and SHA-256 series
conflict in the "init" area, it probably is time for me to introduce
you to Brian (cc'ed) to pick his brain ;-).

Thanks.