Re: [PATCH v2 07/21] config: document `read_early_config()` and `read_very_early_config()`

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 24/08/30 11:09AM, Patrick Steinhardt wrote:
> It's not clear what `read_early_config()` and `read_very_early_config()`
> do differently compared to `repo_read_config()` from just looking at
> their names. Document both of these in the header file to clarify their
> intent.
> 
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
>  config.c |  4 ----
>  config.h | 11 +++++++++++
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/config.c b/config.c
> index 0b87f0f9050..a8357ea9544 100644
> --- a/config.c
> +++ b/config.c
> @@ -2234,10 +2234,6 @@ void read_early_config(config_fn_t cb, void *data)
>  	strbuf_release(&gitdir);
>  }
>  
> -/*
> - * Read config but only enumerate system and global settings.
> - * Omit any repo-local, worktree-local, or command-line settings.
> - */
>  void read_very_early_config(config_fn_t cb, void *data)
>  {
>  	struct config_options opts = { 0 };
> diff --git a/config.h b/config.h
> index d0497157c52..f5fa833cb98 100644
> --- a/config.h
> +++ b/config.h
> @@ -192,7 +192,18 @@ int git_config_from_blob_oid(config_fn_t fn, const char *name,
>  void git_config_push_parameter(const char *text);
>  void git_config_push_env(const char *spec);
>  int git_config_from_parameters(config_fn_t fn, void *data);
> +
> +/*
> + * Read config when the Git directory has not yet been set up. In case
> + * `the_repository` has not yet been set up, try to discover the Git
> + * directory to read the configuration from.
> + */
>  void read_early_config(config_fn_t cb, void *data);

To restate in my own words, `read_early_config()` allows a config to be
read before `the_repository` is setup by discovering the git dir itself.
Out of curiousity, what prevents us from just ensuring `the_repository`
is properly setup first?

> +
> +/*
> + * Read config but only enumerate system and global settings.
> + * Omit any repo-local, worktree-local, or command-line settings.
> + */
>  void read_very_early_config(config_fn_t cb, void *data);

Here `read_very_early_config()` looks like it only cares about system
and global configuration so it doesn't require a git dir or
`the_repository` to be set up. Makes sense.

Not really related to this change, but it would be nice if the name of
the function itself was more descript. Something like
`config_read_system_and_global()`.

Overall, I find these new comments to be very helpful. Thanks! :)




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux