Subject: Re: [PATCH 02/44] repository.c: move env-related setup code back to environment.c

From: Brandon Williams <[email-masked]>
Date: Wed, 21 Mar 2018 14:53:15 -0700

On 03/19, Duy Nguyen wrote:
> On Mon, Mar 19, 2018 at 7:07 PM, Jonathan Tan <[email-masked]> wrote:
> >> -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.
> >    */
> 
> Yeah. I think Eric made the same comment. I'm still (very slowly) in
> the process of unifying the repo setup for the main repo and
> submodules, which hopefully may kill this function or replace it with
> something better. But it's too early to tell. Since this part of the
> series has landed in 'next', I'll post a fixup patch soon with your
> suggestion.
> -- 
> Duy

Yeah looking at the patch this is probably my only complaint.  The rest
looked good.

-- 
Brandon Williams