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(a)jboss.org
>
twitter.com/maniksurtani
>
> Lead, Infinispan
>
http://www.infinispan.org
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
>
https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)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