On 03/19, Duy Nguyen wrote:
> On Mon, Mar 19, 2018 at 7:07 PM, Jonathan Tan <jonathanta...@google.com>
> 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
