Re: [PATCH v9 07/37] hook: add 'run' subcommand

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 3, 2021, 22:29 UTC
Message-ID: <xmqqeedi4m6o.fsf@gitster.g>
In-Reply-To: <875yyv71lz.fsf@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 24 quoted lines

> On Wed, May 26 2021, Emily Shaffer wrote:
>
>> +void run_hooks_opt_init(struct run_hooks_opt *o)
>> +{
>> +	strvec_init(&o->env);
>> +	strvec_init(&o->args);
>> +	o->run_hookdir = configured_hookdir_opt();
>> +}
>
> I suggested in
> https://lore.kernel.org/git/87y2bs7gyc.fsf@evledraar.gmail.com/ that
> this could and should be a RUN_HOOKS_OPT_INIT
>
> After some digging I see that was the case in an earlier version of your
> series, i.e. before:
> https://lore.kernel.org/git/20210131042254.1032233-1-jonathantanmy@google.com/
>
> You came up with this current pattern because of
> configured_hookdir_opt().
>
> But a better option here is to use a RUN_HOOKS_OPT_INIT still and just
> defer the initialization of this "run_hookdir" member. I.e. set it to
> "we have not asked the config yet" in the initializer. I.e. the diff on
> top your series at the end of this E-Mail[1].

When I compared the result of applying your 31-patch series to 2.32-rc2 and the result of rebasing this series on the same base, before sending out a responce to Emily's reaction, I found that the 31-patch series did a good job of not stepping on the "hook defined with configuration" part and concentrated on providing a clean base to build on with a better structure in the series, and there weren't many changes that overlapped with Emily's series in a significant way. The above was one of the "overlapping" differences that stood out.

Thanks.