git hooks for reviews mandatory?

[Kevin Ottens]

On Saturday 21 June 2014 11:22:28 Michael Pyne wrote:
> On Thu, June 19, 2014 23:21:22 Marco Martin wrote:
> > Hi all,
> > I was thinking, since the policy for committing in frameworks is to always
> > asking for a review, what about on repositories under frameworks/* adding
> > an hook that accepts pushes only if the comment has a REVIEW: line?
> > 
> > I have been guilty too many times of not respecting that, mostly for not
> > thinking about it at all, maybe I'm not the only one, an artificial
> > enforce
> > of discipline like that *may* make sense.
> > 
> > opinions? would be useful, or mostly just an annoyance?
> 
> I think it would be mostly an annoyance, but if it were possible to override
> (REVIEW:IRC, REVIEW:TrustMe, etc.) in situations where a Reviewboard
> request is unneeded it could help with ensuring we don't accidentally
> commit something needing review.

Exactly why I think we need to support:
 * REVIEW: <reviewboard id>
 * Reviewed-By: <free string>

The free string then allow to specify "trust me" or a name.

I don't think there's much to do in a hook apart from checking at least one of 
the two is present.

> We'd also want to make sure to come up with pre-commit hooks for devs to use
> client side, or at least a git-commit template reminding to use an
> appropriate REVIEW keyword so that devs don't have to wait until they try
> to push to figure out their commit can't land as-is.

Sounds harder than it sound, I think quite some people rely on the following 
workflow for contributing patches:
 * commit locally;
 * use a script to pick the commit and push it on reviewboard;
 * reword the commit log to add the newly allocated reviewboard id.

Which means the initial commit can't have a REVIEW tag yet... chicken and egg 
problem AFAICT.

Regards.
-- 
Kévin Ottens, http://ervin.ipsquad.net

KDAB - proud supporter of KDE, http://www.kdab.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140621/484396ef/attachment.sig>