Re: [PATCH v2 1/4] promisor-remote: read partialClone config here

> Jonathan Tan <jonathantanmy@google.com> writes:
> 
> > Currently, the reading of config related to promisor remotes is done in
> > two places: once in setup.c (which sets the global variable
> > repository_format_partial_clone, to be read by the code in
> > promisor-remote.c), and once in promisor-remote.c. This means that care
> > must be taken to ensure that repository_format_partial_clone is set
> > before any code in promisor-remote.c accesses it.
> 
> The above is very true, but I am puzzled by the chosen direction of
> the code movement.
> 
> Given that the value in the field repository_format.partial_clone
> comes from an extension, and an extension that is not understood by
> the version of Git that is running MUST abort the execution of Git,
> wouldn't it be guaranteed that, in a correctly written program, the
> .partial_clone field must already be set up correctly before
> anything else, including those in promissor-remote.c, accesses it?
> 
> > To simplify the code, move all such config reading to promisor-remote.c.
> > By doing this, it will be easier to see when
> > repository_format_partial_clone is written and, thus, to reason about
> > the code. This will be especially helpful in a subsequent commit, which
> > modifies this code.
> 
> So, I am not sure if this simplifies the code the way we want to
> read our code.  Doing a thing in one place is indeed simpler than
> doing it in two places, but it looks like promisor-remote code
> should be using the repository-format data more, not the other way
> around, at least to me.
> 
> Perhaps I am missing some other motivation, though.
> 
> Thanks.