Subject: Re: [PATCH v2 00/10] Convert remaining hooks to hook.h

From: Adrian Ratiu <[email-masked]>
Date: Tue, 21 Oct 2025 19:34:16 +0300

On Tue, 21 Oct 2025, Patrick Steinhardt <[email-masked]> wrote:
> On Fri, Oct 17, 2025 at 05:15:34PM +0300, Adrian Ratiu wrote: 
>> Hello everyone,  This is v2 of the series which converts the 
>> remaining hooks to the new API.   I addressed all the feedback 
>> received in v1, with two small exceptions (the ones starting 
>> with "Opted not to" in the below Changes list).   I had a minor 
>> conflict with an upstream change [1] which was trivial to fix. 
>> I added 1 new commit and squashed together two commits (the 
>> simplified update hooks), so in total it's still 10 patches. 
>> The plan is to follow this up with another series which enables 
>> config-based hooks and parallel hook execution, where possible. 
>> As always this is based on the latest master branch, I've 
>> pushed it to GitHub and ran the CI pipeline [3]. The Win+Meson 
>> "missing libgitcore.a" and doc "invalid escape sequence" 
>> failures seem to be unrelated, since I get them without these 
>> patches.   1: 
>> https://github.com/git/git/commit/22e7bc801cd9c5e5b5c4489b631be28e506fec42 
>> 2: 
>> https://github.com/10ne1/git/tree/dev/aratiu/hooks-conversion-v2 
>> 3: https://github.com/10ne1/git/actions/runs/18593709082 
>> Changes between v1 -> v2: * Added a new commit with a mechanism 
>> to override ungroup options (Junio) * Addded a BUG if hook 
>> path_to_stdin and feed_pipe are both provided (Junio) * The 
>> feed_pipe cb can be set independently from path_to_stdin 
>> (Junio) * Simplified the post-rewrite callback (Patrick, 
>> Phillip and Junio) * Document that hook caller owns the 
>> feed_pipe_ctx (Junio) * Removed unnecessary "child" -> 
>> "notes_cp" renames (Phillip) * Reuse strbuf inside pre-push cb 
>> to avoid multiple alloc (Phillip) * Simplified pre-push hook cb 
>> logic (Phillip) * Rewrote reference-transaction cb logic to 
>> mirror pre-push (Patrick) * Simplified the update hook cb by 
>> removing the keepalive logic (Emily) * Squashed the simplified 
>> update and post-update conversions * Iterator types, if 
>> conditions and other small fixes (Patrick) * Fixed a conflict 
>> in refs.c with an upstream for loop sign compare check * Opted 
>> not to use -1 to signify no fd value instead of 0, because I'd 
>> have to 
>>   significantly rework the run-command.h .in/.out/.err API 
>>   (Patrick) 
>> * Opted not to move sigchain_push(SIGPIPE, SIG_IGN); into 
>> pp_buffer_stdin()) 
>>   because it will called too many times inside the process loop 
>>   (Patrick) 
>> * Added Helped-by: Emily Shaffer tag to the 
>> reference-transaction coversion * Comments, typos, stray lines, 
>> commit rewordings (Ben, Patrick, Emily, Junio) 
> 
> By the way, it would have helped to have a range-diff here so 
> that it's easy to see what exactly changed between the two 
> different versions.  Could you maybe include that in subsequent 
> iterations? 
 
Sure, I will include a range-diff going forward.

Thank you again for your very awesome feedback,
Adrian