git hooks for reviews mandatory?

Kevin Ottens ervin at kde.org
Fri Jun 20 05:40:14 UTC 2014


On Friday 20 June 2014 01:46:10 Aleix Pol wrote:
> On Thu, Jun 19, 2014 at 11:21 PM, Marco Martin <notmart at gmail.com> 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've heard of many complaints about how noisy is kde-frameworks mailing
> list because of review requests.

Well, the policy doesn't necessarily mean going through reviewboard. As 
pointed by Luigi "Reviewed by:" is also allowed, and that can be through a 
pastebin over IRC or pair programming or whatever you want to get something 
more synchronous and direct as a review. Reviewboard is really for the 
asynchronous case (no relevant people available) or "I'm not sure what I'm 
doing" case. The rest could be done through other means of reviews.

> Also, I fear there's people not working at
> full speed in their respective projects because of review requests.

Yeah, let's get crap out at full speed. :-)

I think I see what you mean. That said, I'd like to remind being careful with 
the "full speed" argument.

>  Such change would scare me a bit, TBH.

If we limit it to "REVIEW:" then it would concern me as well as it'd force too 
much of a workflow. If we support both "REVIEW:" and "Reviewed by:" then it is 
less of a concern IMO.

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/20140620/ccf81318/attachment.sig>


More information about the Kde-frameworks-devel mailing list