Re: [PATCH v2 4/6] config: don't implicitly use gitdir

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

 



Brandon Williams wrote:

> Commit 2185fde56 (config: handle conditional include when $GIT_DIR is
> not set up) added a 'git_dir' field to the config_options struct.  Let's
> use this option field explicitly all the time instead of occasionally
> falling back to calling 'git_pathdup("config")' to get the path to the
> local repository configuration.  This allows 'do_git_config_sequence()'
> to not implicitly rely on global repository state.
>
> Signed-off-by: Brandon Williams <bmwill@xxxxxxxxxx>
> ---
>  builtin/config.c | 2 ++
>  config.c         | 6 ++----
>  2 files changed, 4 insertions(+), 4 deletions(-)

The same comments as before still apply:

- this changes API to make opts->git_dir mandatory, which is error prone
  and easily avoidable, e.g. by making git_dir an argument to
  git_config_with_options

- the commit message doesn't say anything about to git dir vs common dir
  change.  It needs to, or even better, the switch to use common dir
  instead of git dir can happen as a separate patch.

Thanks,
Jonathan



[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]