Re: [PATCH v2 0/1] refs: add 'preparing' phase to the reference-transaction hook
- From
Peijian Ju <eric.peijian@gmail.com>
- Date
- Mar 16, 2026, 23:08 UTC
- Message-ID
- <CAN2LT1DJcSEKuQOk2PHgUwORKwR4Vqo5=f2_FtNXHMH0BxvLZQ@mail.gmail.com>
- In-Reply-To
- <aberRbSCbMtZrqxk@pks.im>
On Mon, Mar 16, 2026 at 3:03 AM Patrick Steinhardt <ps@pks.im> wrote:
Show 43 quoted lines
>
> On Mon, Mar 16, 2026 at 12:51:01AM -0400, Eric Ju wrote:
> > Changes since v1:
> >
> > - Fix commit title to follow "area: description" convention
> > ("refs: add 'preparing' phase to reference-transaction hook")
> > - Correct phase names in documentation to past tense
> > ("committed", "aborted")
> > - Fix the sentence about backwards compatibility with unknown phases
> > - Update die() messages to identify the hook by full name and phase
> > ("ref updates rejected by the reference-transaction hook at its
> > preparing/prepared phase")
> > - Consolidate author identity to eric.peijian@gmail.com
> > - Add clarification in reply to the question about how to use the preparing
> > phase for write serialization
>
> All of these changes look good to me, thanks. This patch already looks
> good to me, but I'm of course biased as I have been helping out behind
> the scenes before the first version of this patch landed on the mailing
> list.
>
> > Range-diff against v1:
> > 1: 5f9f13a84d ! 1: fb74f21d98 Add preparing state to reference-transaction hook
> > @@ Commit message
> > interfering with the locking state.
> >
> > This change is strictly speaking not backwards compatible. Existing hook
> > - scripts that do not know to handle unknown phases handle the "preparing" state
> > - string will encounter an unknown phase, and that might cause them to return an
> > - error now. But the hook is considered to expose internal implementation details
> > + scripts that do not know how to handle unknown phases may treat
> > + 'preparing' as an error and return non-zero.
> > + But the hook is considered to expose internal implementation details
> > of how Git works, and as such we have been a bit more lenient with changing its
> > exact semantics, like for example in a8ae923f85 (refs: support symrefs in
> > 'reference-transaction' hook, 2024-05-07).
>
> One micro-nit: this paragraph could use some reflowing. But I don't
> think it's worth a reroll.
>
> Thanks!
>
> PatrickThank you. I will reflow the paragraph in v3, which I am already planning to send for the error message and string constant changes.
- Eric