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

Mircea Markus mircea.markus at jboss.com
Fri May 25 09:44:52 EDT 2012


> 
> 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.

> 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




More information about the infinispan-dev mailing list