Re: [PATCH 02/44] repository.c: move env-related setup code back to environment.c
- From
- Brandon Williams <bmwill@google.com>
- Date
- Mar 21, 2018, 21:53 UTC
- Message-ID
- <20180321215315.GB18406@google.com>
- In-Reply-To
- <CACsJy8D3Dn8FeSX9su8edYFM28N-qG2bjNYRpajNWaEi6qdTDQ@mail.gmail.com>
On 03/19, Duy Nguyen wrote:
Show 31 quoted lines
> On Mon, Mar 19, 2018 at 7:07 PM, Jonathan Tan <jonathantanmy@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.
> --
> DuyYeah looking at the patch this is probably my only complaint. The rest looked good.
-- Brandon Williams