From: Chandra <chandrakr@pm.me>
Date: Thu, 05 Mar 2026 13:46:16 GMT
Subject: Re: [PATCH v5] add: support pre-add hook
Message-ID: <i6rBoZXrLVKU8Yc6UtDNpPB0KJ8dbQlyGzxKF_ofhIiSfScuKHcJHewPAmWX3kiF5vr9uNuSQkJg7NbCV9VEKfMs-ez09VKhfMX8u84n5qU=@pm.me>
In-Reply-To: <87o6l2xuku.fsf@collabora.com>

Hi Adrian,

Yes, I used Claude CLI to suggest changes in a markdown file and then added some changes from there myself. Claude suggested a test to do what you asked here, but I thought it redundant since after the hook is discovered everything should work the same. Claude agreed, but AI tends to agree when you push back on most things. Please let me know if my intuition here about test redundancy was wrong and I should go ahead with adding that exercise.


Chandra Kethi-Reddy
@archonphronesis:matrix.org

Sent from Proton Mail for iOS.

-------- Original Message --------
On Thursday, 03/05/26 at 19:11 Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
Hi again Chandra,

On Thu, 05 Mar 2026, "Chandra Kethi-Reddy via GitGitGadget" <gitgitgadget@gmail.com> wrote:
> Range-diff vs v4:
>
>  1:  9383395bb0 ! 1:  fc58c4cba2 add: support pre-add hook
>      @@ builtin/add.c: int cmd_add(int argc,
>        		string_list_clear(&only_match_skip_worktree, 0);
>        	}
>
>      -+	if (!show_only && !no_verify && find_hook(repo, "pre-add")) {
>      ++	if (!show_only && !no_verify && hook_exists(repo, "pre-add")) {
>       +		run_pre_add = 1;
>       +		orig_index_path = absolute_pathdup(repo_get_index_file(repo));
>       +	}
>      @@ t/t3706-pre-add-hook.sh (new)
>       +	git commit -m "initial"
>       +'
>       +
>      ++test_expect_success 'hook found via core.hooksPath' '
>      ++	test_when_finished "git reset --hard &&
>      ++			    rm -rf custom-hooks &&
>      ++			    git config --unset core.hooksPath" &&
>      ++	mkdir custom-hooks &&
>      ++	write_script custom-hooks/pre-add <<-\EOF &&
>      ++	echo invoked >hook-ran
>      ++	EOF
>      ++	git config core.hooksPath custom-hooks &&
>      ++	echo changed >>file &&
>      ++	git add file &&
>      ++	test_path_is_file hook-ran &&
>      ++	rm -f hook-ran
>      ++'

The test you added is rather surprising, was it written by Claude AI?

For clarification, what I asked for is to add tests which define the new
hook via configs like done in t1800-hook.sh tests, for example in your
case, you can define a simple test like this:

test_config hook.my-friendly-echo.event "pre-add" &&
test_config hook.my-friendly-echo.command "echo hello from hook" &&

See Documentation/config/hook.adoc for more details.

The turnaround in minutes between v4 -> v5 is also surprising.
Please give humans a chance to review & respond, at least a couple of
days between resvisions. :)

Thanks,
Adrian