Re: Patches for git-push --confirm and --show-subjects

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 15, 2009, 05:50 UTC
Message-ID: <7v1vm892ow.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1252982329.11581.111.camel@localhost.localdomain>

Owen Taylor <otaylor@redhat.com> writes:

Show 19 quoted lines

> On Mon, 2009-09-14 at 17:46 -0700, Junio C Hamano wrote:
>> Owen Taylor <otaylor@redhat.com> writes:
>> 
>> > If I can figure out the rest of it, I'll look at adding a hook on top as
>> > a sweetener :-)
>> 
>> Please don't.
>> 
>> I seriously suggest you start from, and stick to, nothing but a hook.
>> 
>> The pre-push codepath is conceptually very simple --- something needs to
>> inspect a list of <ref, old, new> and say yes or no.  But what the users
>> want needs great customizability (e.g. Daniel's sign-off validation
>> example).  It's the prime example of codepath that should have a hook and
>> no built-in policy logic.
>
> Let me back up on this a little bit.
>
> Is confirmation a general need?

If you limit it to the confirmation alone, the answer is probably "not necessarily". But a mechanism to allow validation logic to be plugged in probably is.

You might not see a "policy" in your approach, but it makes some troubling hardcoded policy decisions. Here are a few examples of what your patch decides, and makes it harder for other people to build on (rather, "around):

 - We support only interactive validation (confirmation).  If you want to
   have an unattended validation scheme, there is no way to enhance the
   mechanism this patch adds to do so.  You instead need to add yet
   another command line option and hook into the same place as this patch
   touches.

 - We assume "git push" is run from terminal, and the only kind of
   interactive validation we support is via typed confirmation from a line
   terminal "[Y/n]?"  If you want to run "git push" from a GUI frontend
   and have the user interact with a dialog window popped up separately,
   you are also out of luck.

 - We assume it is good enough to have various built-in presentations of
   supporting information while asking for confirmations; there is no way
   for casual end users to customize and enhance it.

I honestly do not want to be a part of "We" in the above bullet points.

I do not object to having a good default presentation and default interaction (assuming for a while that we limit ourselves only to "interactive confirmation"). But that is a very different matter from closing the door for other possibilities, which is essentially what the approach to use built-in policy logic that is configurable with unbounded number of future command line options to "git push" is.

> Providing a gnome-contributor-git-setup.sh is generally an approach of
> last resort.

No question about that. We do not have any complex built-in policy code that is triggered at post-receive time at all, but many people use the sample post-receive-email hook we ship unmodified in their repositories, because the script is written in a highly configurable way. I do not see why pre-push has to be any different.

In any case, this topic won't be part of 1.6.5, and we have plenty of time to prototype and polish it before it goes to the end user.