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

Reply via email to