Ben Keene <seraphire@xxxxxxxxx> 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.
