Re: [PATCH] githooks.txt: Clarify documentation on reference-transaction hook

On Fri, Feb 26, 2021 at 01:03:43AM -0500, Jeff King wrote:

On Thu, Feb 25, 2021 at 06:50:12AM +0100, Patrick Steinhardt wrote:

quoted

The reference-transaction hook doesn't clearly document its scope and
what values it receives as input. Document it to make it less surprising
and clearly delimit its (current) scope.

Signed-off-by: Patrick Steinhardt <redacted>
---

I've been postponing doing this simple doc update for far too long, but
here it finally is. It simply clarifies its current workings and
limitations without changing anything. This is not supposed to be a "We
don't want it to ever cover symrefs", but rather to avoid confusion.

I think that's a good step forward. We might want to say "does not cover
symbolic references (but that may change in the future)" to make it
clear that nothing is definite.

Fair, I'll add that.

OTOH, I suspect adding them would require a change to the hook's stdin
format, so it is not like a hook could be written in a way to magically
handle them if things change in the future.

Yeah. Hindsight is 20/20, but this should've used some kind of prefix
with the explicitly stated hint that additional prefixed can be added in
the future. I've got myself to blame for that, I should've known better
to make this more readily extensible.

quoted

@@ -492,6 +493,13 @@ receives on standard input a line of the format:
 
   <old-value> SP <new-value> SP <ref-name> LF
 
+where `<old-value>` is the old object name passed into the reference
+transaction, `<new-value>` is the new object name to be stored in the
+ref and `<ref-name>` is the full name of the ref. When force updating
+the reference regardless of its current value or when the reference is
+to be created anew, `<old-value>` is 40 `0`. To distinguish these cases,
+you can inspect the current value of `<ref-name>` via `git rev-parse`.

We should probably avoid saying "40" here. Maybe "all zeroes" or
something.

-Peff

I wasn't completely sure how to word this and thus opted to do the same
as the other hooks. Most notably, both pre-push and pre-receive hook
explicitly call out SHA-1 and the 40-character thingy.

Maybe I'll just add a patch on top to also change those to instead say
"the all-zero object ID" or something similar.

Patrick