From: Chandra <chandrakr@pm.me>
Date: Thu, 05 Mar 2026 12:37:45 GMT
Subject: Re: [PATCH v4] add: support pre-add hook
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:

> 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)
> 
>