Re: [PATCH 1/1] add hook pre-p4-submit

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 25, 2018, 20:21 UTC
Message-ID: <xmqqin53ukvc.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAPig+cR2gYEwOTVBMRde35rn9oVsixeerbm5iJV+FmnOiBWxqQ@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

> Bikeshedding: Should this be named 'p4-pre-submit'? That way, if
> git-p4 ever grows additional hooks, they can all be grouped under the
> "p4-" prefix.

An excellent suggestion.

Show 9 quoted lines

> Does the hook receive any arguments? Does it receive anything on its
> standard input? Those questions ought to be answered by the
> documentation.
>
> Also, how is the hook supposed to determine whether the "submit"
> should proceed? How does it know what is being submitted? Perhaps the
> documentation could provide some explanation of how the hook should
> glean such information (especially if the hook does not itself receive
> any input).

Yes, such information in the documentation, or lack thereof, would mean the distinction between a useful feature and an unusable one.

All other review comments looked quite sensible and an update should address all of them.

Thanks for being a careful reviewer, as always.