Re: [RTC/PATCH] Add 'update-branch' hook

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Apr 24, 2014, 01:00 UTC
Message-ID: <535862411c29d_3c7abff3103e@nysa.notmuch>
In-Reply-To: <xmqqvbtzwv4l.fsf@gitster.dls.corp.google.com>

Junio C Hamano wrote:

Show 17 quoted lines

> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Junio C Hamano wrote:
> >> Felipe Contreras <felipe.contreras@gmail.com> writes:
> >> 
> >> > ... there are _already_ hooks without pre/post.
> >> 
> >> Like commit-msg?  Yes, it would have been nicer if it were named
> >> verify-commit-message or something.
> >
> > No it wouldn't. I can use the commit-msg hook to change the commit message and
> > to absolutely no verification, so verify-commit-message would be misleading.
> 
> You are confused (and please do not spread the confusion).  If you
> read the first paragraph of the documentation on the hook and think
> for 5 seconds why "--no-verify" countermands it, you would realize
> that the hook is primarily meant for verification.

I do not care what the hook is "primarily for", it's for more than just verification.

> We also allow the hook to edit the message, but that is not even "a useful
> feature added as an afterthought"; the documentation mentions it because the
> implementation did not bother to make sure the hook did not touch the message
> file.

Indeed it's too late now, and now the hook does more than just verification, therefore verify-commit-message wouldn't be an appropriate name.

> It was a mistake not to call it with a clear name that tells
> verification happens there.

No, the name is fine for what the hook does, if you would want the script to do something different, *and* change the name of the script, that's a different issue.

Show 5 quoted lines

> >> Old mistakes are harder to change because of inertia.  It is not a
> >> good excuse to knowingly make a new mistake to add new exceptions
> >> that the users need to check documentations for, is it?
> 
> I see no reason to waste more time on this point.

You haven't proved it's a mistake.

The only thing you have showed is that letting the 'commit-msg' modify the message was a mistake, not that the name is wrong for what it currently does.

-- 
Felipe Contreras