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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
> 
> > Junio C Hamano wrote:
> >> Felipe Contreras <felipe.contreras@xxxxxxxxx> 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.

> >> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]