Re: [PATCH v2 00/10] Convert remaining hooks to hook.h
- From
Adrian Ratiu <adrian.ratiu@collabora.com>
- Date
- Oct 21, 2025, 16:34 UTC
- Message-ID
- <87ms5kp62f.fsf@collabora.com>
- In-Reply-To
- <aPc4_Pyd37epd0j4@pks.im>
On Tue, 21 Oct 2025, Patrick Steinhardt <ps@pks.im> wrote:
Show 50 quoted lines
> 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