[OpenLayers-Dev] Commits without review (Was: #1218: Serialization doesn't wrap nodes in documents)

[Christopher Schmidt]

On Tue, Dec 18, 2007 at 03:13:30PM -0700, Tim Schaub wrote:
> Great.  Can you try to capture some of your ideas for a new process on a 
> wiki page?  As I mentioned from the start, I'm eager to get a process 
> down that we all agree with.  I also mentioned that I think the 
> following three items are a recipe for disaster:
> 
> 1) commits without review
> 2) more committers
> 3) frequent releases
> 
> As I said before, I'm in favor of 2 and 3.  Your message subject 
> suggests 1.  Since I think those three items conflict, I'm just asking 
> you to give some more detail on what you would like implemented.

I guess I don't understand how things like #1082 would impact 2 or 3.

Problem statement: Changes take too long to make it into trunk. 
Specific subset of the problem that i'm interested in: the reason for
this is not bad code or things which are otherwise lacking, but instead,
lack of reviewers.

I see two possible solutions.

 1. small commits without review. Definition of small may vary. I'd
    take the xml format serializer as an example of an upper bound on
    small.
 2. Somehow get more people reviewing.

The former helps to accelerate reviews because the small things 'go
away' after some time period of no one objecting. This works in cases
where API methods are not affected, because committers still have the
ability to object. Even API Methods have this  attribute until a
release.

Adding more committers does not -- at least in the short run -- get us
more reviews, because new committers are unlikely to take initiative to
mark something as reviewed. (Understandably so.) So I don't actually see
ho to change #2 -- and adding more committers only increases our patch
backlog until we get more reviewers. 

So, your #2 above would make the situation worse: I'm already reviewing
everything I can, and things are still sitting for several months
without any activity, includig patches contributed by non-committer
users who are likely to just go away when they find no feedback from the
community on their patches.

So, a possible direction:
 * Everything still gets a ticket.
 * Patch is uploaded to the ticket.
 * Assuming the patch neither adds nor modifies function signature of
   any method, and no objections are voiced after some time period, the
   ticket can go in.

This helps to prevent the large patch backlog we have now -- where I'm
concentrating more on making sure my patches are still moving forward
than reviewing other patches.

Is there some other way to increase the number of patches that go
through? I'm assuming that others aren't revieing because they have no
time: I try very hard to keep the
http://trac.openlayers.org/wiki/Release/2.6 page inline, which means
that there is always a 'list of things which need review' which easily
accessible, so I'm assuming more information in that direction doesn't
actually help. 

Why aren't people reviewing? Fear of retribution from on high? Not enough
time? Not enough insight into the project?

Maybe there's an obvious solution that I'm missing. I have always
assumed that our review process simply assigns too much responsibility,
and therefore committers are unwilling to participate iwthout
deep/intimate knowledge. Certainly I feel like every time I commit
something which adds an APIMethod or APIProperty without first reviewing
it with Tim or Erik, I have done something wrong, so I simply don't
review most things, and instead try to draw attention to tickets to
gather feedback, typically unsuccessfully. 

I feel like I spend 6-8 hours every weekend on working on patches,
reviews, etc. and never move the OpenLayers codebase forward in any
significant way. I'd like to change that. I don't know what the solution
is, but I'm worried that if I feel this way, other contributors who
aren't committers must be feeling that way to a much greater extent.

Regards,
-- 
Christopher Schmidt
MetaCarta