On 01/31/2010 12:13 AM, John Plevyak wrote:
I started out by posting all my checkins as patches.
I got few comments/reviews which, in the interest of progress,
I took as approval.
Yes, those were approvals.
Here is my suggestion. We need to have folks claim modules and that
means promise to review patches in a timely manner (say a week).
100% agree. The system "fell apart" for two reasons: 1) Reviews were not
happening in a timely manner 2) In some cases, I got the impression that
"dev" branch was considered experimental and not under normal review
rules. It is (or was) up to each committer to request (and hunt down)
someone for a review. In many cases, this did happened and worked, but
sometimes things fell through the crack.
As such, I'd like to make the above a rule. If a patch has been
proposed, and the proposer requested reviews properly, if it's not
reviewed within two weeks (I think one week is a bit aggressive, not
allowing for vacation etc.), an implicit approval for checkin is
granted. We should strongly recommend that reviews happens within a week
though, if at all possible.
How about this: I will review all changes to the
iocore modules: eventsystem/net/aio/cache/cluster/hostdb/dns/libinktomi++,
and, so long as I am in town, I will do it within a week.
Great! We need to make a Wiki page with module owner information, we
obviously need a bunch of people willing to review code. I think we
should aim to have 2 reviewers per larger piece of code base, maybe not
initially, but long term. Also, any committer should consider
him/herself empowered to review any code.
Finally, I hope no one took my email the wrong way, I'm as much (or
more) at "fault" here as anyone, so lets work together to get the
process to work like we want it to.
Cheers!
-- Leif
