> From: Junio C Hamano <gitster@xxxxxxxxx>
> > From: Sean Barag <sean@xxxxxxxxx>
> >
> > While Junio's request [1] was to avoids the unusual "write config
> > then immediately read it" pattern that exists in `cmd_clone`,
> > Johannes mentioned that --template can write new config values that
> > aren't automatically included in the environment [2]. This requires
> > a config re-read after `init_db` is called.
> >
> > Moving the initial config up does allow settings from config to be
> > overwritten by ones provided via CLI options in a more natural way
> > though, so that part of Junio's suggestion remains.
>
> The title says what the code does after this change. The code calls
> git_config() before calling parse_options(), but not much in the
> proposed log message explains what the patch tries to achieve by doing
> so.
>
> The above refers to suggestions but does not describe what problem the
> patch is trying to address and what approach is taken to address it.
Thanks for the pointer - I completely agree. Rewriting for v2.
> > +static int git_clone_config(const char *k, const char *v, void *cb)
> > +{
> > + return git_default_config(k, v, cb);
> > +}
> > +
> > static int write_one_config(const char *key, const char *value, void *data)
> > {
> > + /*
> > + * give git_config_default a chance to write config values back to the environment, since
> > + * git_config_set_multivar_gently only deals with config-file writes
> > + */
>
> Overlong lines...
How embarrassing! Re-wrapped for v2.
> > + int apply_failed = git_default_config(key, value, data);
>
> Not git_clone_config()? Presumably you'll make git_clone_config()
> recognise more variables than git_default_config() does, and the
> caller of this helper wants us to recognise "clone.*" that are
> ignored by git_default_config() callback, no?
Great catch! This gets changed to `git_clone_config` in patch 4/4
anyway, so there's no need for the confusing intermediate state in 2/4.
Will be fixed in v2.
--
Thanks for being so patient with a newbie :)
Sean
