[infinispan-dev] Let's stop with pull requests

[Martin Gencur]

Mircea Markus píše v Fri 25. 05. 2012 v 14:44 +0100:
> > 
> > I'll take over that role.  One thing I'd like to propose:
> > 
> > 1.  The reviewer is ultimately responsible for the patch.  If a broken patch makes it to upstream it is the reviewer's fault.
> 
> I think we should differentiate from  community patches (i.e. external contributors) and internal patches (core team).
> Your suggestion is a must for community patches, but I think for internal patches it should be the patch provider that has the responsibility of running the test and the review process should only be there for a second opinion. The tests should rather be run by the person that *can fix them* - and that's the patch provider - otherwise we introduce an unnecessary loop in communication. Also review takes much longer, which puts people off when it comes to doing it.

There might be cases when the testsuite passes for the contributor but
then it brakes the master branch because another patch was pushed
upstream in the meantime which changed e.g. some API that the
contributor had been using. 

So the reviewer should IMO take care of it by running the testsuite or
pushing the commits in the appropriate order. 

> 
> > 2.  The patch author is responsible for wasting reviewer's time with bad patches.
> > 
> > I think we should make a game of this: we have a "bad commits" jar.  Each time a reviewer pushes a bad patch up, he needs to put $1 into the jar.  The next time, $(1 << 2), the next $(1 << 3), etc.  :)
> > 
> > To prevent incomplete pull requests wasting reviewer's time, the reviewer is allowed to impose a similar penalty on the patch  author.  Naturally, this only covers blatantly bad patches - i.e., incomplete tests, broken builds - and not genuine feedback on the patch, etc.
> > 
> > And of course the "bad commits jar" is used to buy beer the next time we all meet.  Inspired by a tweet Sanne fwd'd a while back.  ;)
> > 
> > If we're happy with this, I'll maintain the accounting spreadsheet for this bad commits jar.
> > 
> > - M
> > --
> > Manik Surtani
> > manik at jboss.org
> > twitter.com/maniksurtani
> > 
> > Lead, Infinispan
> > http://www.infinispan.org
> > 
> > 
> > 
> > 
> > _______________________________________________
> > infinispan-dev mailing list
> > infinispan-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
> 
> 
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

-- 
Martin Gencur
--
QA Engineer, JBoss Data Grid
Desk phone: +420 532 294 192, ext. 62192