Re: [PATCH v4] add: support pre-add hook
- From
- Chandra <chandrakr@pm.me>
- Date
- Mar 5, 2026, 12:37 UTC
- Message-ID
- <TqpXjikveTe2dR39_ZEgb0bz0KLLFtaHN_Exd-wwOUAR2RkfcuWBnPUY7wvsTDLaISn_a-cfzssvflKTr_5lSIisfLG6OvGmdEg9gNTIbng=@pm.me>
- In-Reply-To
- <87seaexz33.fsf@gentoo.mail-host-address-is-not-set>
Hi all,
Thanks for the review, Adrian. v5 addresses both points you brought up. Also, I fixed a failing test on windows CI with proper path formatting.
Chandra Kethi-Reddy @archonphronesis:matrix.org
Sent with Proton Mail secure email.
On Thursday, March 5th, 2026 at 5:44 PM, Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
Show 24 quoted lines
> Hi Chandra,
>
> On Thu, 05 Mar 2026, "Chandra Kethi-Reddy via GitGitGadget" <gitgitgadget@gmail.com> wrote:
> > @@ -576,6 +582,11 @@ int cmd_add(int argc,
> > string_list_clear(&only_match_skip_worktree, 0);
> > }
> >
> > + if (!show_only && !no_verify && find_hook(repo, "pre-add")) {
> > + run_pre_add = 1;
> > + orig_index_path = absolute_pathdup(repo_get_index_file(repo));
> > + }
> > +
>
> Please use hook_exists() instead of find_hook() because that works with
> hooks defined via config files. Otherwise your hooks API usage is great.
>
> Maybe add a test or two which define the pre-add hook via configs to
> verify it works?
>
> (regarding find_hook(), we sholud mark it as deprecated or convert all
> its remaining uses and remove it, however that's outside the scope of
> your series, no worries)
>
>