Re: [PATCH] git-p4: Add hook p4-pre-pedit-changelist

[Junio C Hamano <gitster@pobox.com>]

Ben Keene <seraphire@gmail.com> writes:

> ... 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?

>>   * 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 ;-)

>>   * 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).

>>   * 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.