On Sat,  3 Mar 2018 18:35:55 +0700
Nguyễn Thái Ngọc Duy  <pclo...@gmail.com> wrote:

> It does not make sense that generic repository code contains handling
> of environment variables, which are specific for the main repository
> only. Refactor repo_set_gitdir() function to take $GIT_DIR and
> optionally _all_ other customizable paths. These optional paths can be
> NULL and will be calculated according to the default directory layout.
> 
> Note that some dead functions are left behind to reduce diff
> noise. They will be deleted in the next patch.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>

Thanks - I've reviewed up to this patch, and patches 1 and 2 look good.

> -extern void repo_set_gitdir(struct repository *repo, const char *path);
> +struct set_gitdir_args {
> +     const char *commondir;
> +     const char *object_dir;
> +     const char *graft_file;
> +     const char *index_file;
> +};
> +
> +extern void repo_set_gitdir(struct repository *repo,
> +                         const char *root,
> +                         const struct set_gitdir_args *optional);

Optional: Reading this header file alone makes me think that the 3rd
argument can be NULL, but it actually can't. I would name it
"extra_args" and add a comment inside "struct set_gitdir_args"
explaining (e.g.):

  /*
   * Any of these fields may be NULL. If so, the name of that directory
   * is instead derived from the root path of the repository.
   */

Reply via email to