| Message ID | c30d41de55b8991a09e1d550e853f582b5394dee.1614232040.git.ps@pks.im (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | githooks.txt: Clarify documentation on reference-transaction hook | expand |
On Thu, Feb 25, 2021 at 06:50:12AM +0100, Patrick Steinhardt wrote: > 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 <ps@pks.im> > --- > > 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. 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. > @@ -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
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: > > > 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 <ps@pks.im> > > --- > > > > 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. > > @@ -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
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index 1f3b57d04d..b01de04702 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -473,7 +473,8 @@ reference-transaction This hook is invoked by any Git command that performs reference updates. It executes whenever a reference transaction is prepared, -committed or aborted and may thus get called multiple times. +committed or aborted and may thus get called multiple times. The hook +does not cover symbolic references. The hook takes exactly one argument, which is the current state the given reference transaction is in: @@ -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`. + The exit status of the hook is ignored for any state except for the "prepared" state. In the "prepared" state, a non-zero exit status will cause the transaction to be aborted. The hook will not be called with
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 <ps@pks.im> --- 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. Patrick Documentation/githooks.txt | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)