Web lists-archives.com

Re: [PATCH 1/2] run-command: teach 'run_hook' about alternate worktrees




On Mon, Feb 12, 2018 at 3:58 PM, Lars Schneider
<larsxschneider@xxxxxxxxx> wrote:
>> On 12 Feb 2018, at 04:15, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>> +int run_hook_ve(const char *const *env, const char *name, va_list args)
>> +{
>> +     return run_hook_cd_ve(NULL, env, name, args);
>> +}
>
> I think we have only one more user for this function:
>         builtin/commit.c:       ret = run_hook_ve(hook_env.argv,name, args);
>
> Would it be an option to just use the new function signature
> everywhere and remove the wrapper? Or do we value the old interface?

I did note that there was only one existing caller and considered
simply modifying run_hook_ve()'s signature to accept the new 'dir'
argument. I don't feel strongly one way or the other.

However, as mentioned elsewhere[1], I'm still not convinced that this
one-off case of needing to chdir() warrants adding these overloads to
'run-command' at all, so I'm strongly considering (and was considering
even for v1) instead just running this hook manually in
builtin/worktree.c.

[1]: https://public-inbox.org/git/CAPig+cQ6Tq3J=bS8ymDqiXqUvoUiP59T=FGZgMw2FOAx0vyo=Q@xxxxxxxxxxxxxx/