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

> > @@ -99,6 +94,15 @@ static int promisor_remote_config(const char *var, const char *value, void *data
> >         size_t namelen;
> >         const char *subkey;
> >
> > +       if (!strcmp(var, "extensions.partialclone")) {
> > +               /*
> > +                * NULL value is handled in handle_extension_v0 in setup.c.
> > +                */
> > +               if (value)
> > +                       repository_format_partial_clone = xstrdup(value);
> > +               return 0;
> > +       }
> 
> This is actually slightly hard to parse out.  I was trying to figure
> out where repository_format_partial_clone was initialized, and it's
> not handled when value is NULL in handle_extension_v0; it's the fact
> that repository_format_partial_clone is declared a static global
> variable.
> 
> But in the next patch you make it a member of struct
> promisor_remote_config, and instead rely on the xcalloc call in
> promisor_remote_init().
> 
> That means everything is properly initialized and you haven't made any
> mistakes here, but the logic is a bit hard to follow.  Perhaps it'd be
> nicer to just write this as
> 
> +       if (!strcmp(var, "extensions.partialclone")) {
> +               repository_format_partial_clone = xstrdup_or_null(value);
> +               return 0;
> +       }
> 
> which makes the code shorter and easier to follow, at least for me.