Re: [PATCH] git-p4: Add hook p4-pre-pedit-changelist
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 30, 2020, 18:39 UTC
- Message-ID
- <xmqqmua4dc0b.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <1dfe9fba-ec25-cd44-981a-25f7bcba31cc@gmail.com>
Ben Keene <seraphire@gmail.com> writes:
Show 6 quoted lines
> ... the name of the hook from "p4-pre-edit-changelist" to follow the > git standard hooks: > > * "p4-prepare-changelist" - This will replace the proposed hook but still > take only the filename. This hook will be called, even if the > prepare-p4-only option is selected.
With "to follow", I presume that this corresponds to the "prepare-commit-msg" hook? Does "changelist" in P4 lingo roughly correspond to "commit", or to "commit message"?
> * "p4-changelist" - this is a new hook that will be added after the > user edit of the changelist text, but prior to the actual submission. > This hook will also take the temporary file as it's only parameter > and a failed response will fail the submission.
And I presume that this corresponds to "commit-msg" hook?
Show 7 quoted lines
>> * Given that "git commit" has a pair of hooks for log message, is >> adding one new hook a reasonable thing? If so, the log mesasge >> should explain why (e.g. perhaps the other one already is there, >> or perhaps the other one is not applicable in the context of >> interacting with P4 with such and such reasons).) > > I agree with your suggestion.
OK. Let's find them in the updated patch ;-)
Show 6 quoted lines
>> * Is it reasonable not to have a mechanism to disable/skip the >> hook, like "git commit" does? If not, the log message should >> explain why such an escape hatch, which is needed for "git >> commit", is not needed. > The existing hook, p4-pre-submit, does not have an escape hatch, > so I did not add one to this method, but I can certainly add one.
It is probably a good idea in the longer term, but we can certainly punt and then revisit to cover them with --no-verify later (as long as we do document our intention to do so in the log message).
Show 6 quoted lines
>> * githooks(5) manual page is supposed to list all hooks, so a patch >> that adds a new one should add a description for it in there. > > I'll add text for these files (githooks and the git-p4 pages). > > I'll make a new submission soon.
Thanks.