Re: [PATCH] add a 'pre-push' hook

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

"Scott Chacon" <schacon@gmail.com> writes:

> I would be happy to add the name of the branch being updated and the remote
> we're trying to push to.  Is there interest then, in the patch?

I do not see a fundamental reason not to have a pre-push hook that
inspects what is going to be pushed where and declines it.  We already do
"don't push non-fast-forward" on the sending end, and we can think of this
as an exhancement of that.

IOW, I am Ok with the goal.  I haven't looked at the implementation,
though. I seem to recall all of tabstops in the patch were lost somewhere
between your editor and my MUA to make the patch unappliable, and stopped
reading there.

> Should I spend my
> precious brain cycles on adding that functionality?

Well, submitters pushing changes to scratch their own itch don't say "my
precious brain cycles".  Reviewers working with submitter to polish and
accept the change to help the submitter, even when it is not their own
itch, do ;-).

In any case, I think passing the information necessary for the validation
to the hook is essential to make this patch worthwhile.  Without it, I
have to agree with Jeff and Shawn that it is no better than a wrapper.

I merely raised destination URL and the tip that is sent as an example,
but I suspect we may also want to know the name of branch on the remote
side that is being updated, and the old tip commit on that branch as well.

The real issue about "that functionality" is what kind of information is
often needed and/or is enough, and we may need to look at what kind of
validation are useful in expected use scenarios.

For example, if your goal is "to enforce that the tip of the tree is
always without compilation errors", then you only need the commit that you
are sending.  If however that policy applies only to some branches but not
other, you would need to know which branch you are sending things to.  If
you are scanning what is sent to the remote side so that your published
history does not leak confidential information, you further need to know
the old tip and run "rev-list --objects old..new" and inspect all the new
objects you are sending, not just the tip.

I am just listing these off the top of my head; you as the original
submitter of the patch must have thought about the issues and use cases
much deeper than I did in the past 7 minutes while I am typing this
message, so you are in a better position to come up with various use cases
and the set of parameters the hook would need to do its validation job.