[OpenLayers-Dev] Review of patches, etc.

[Tim Schaub]

Hey-

I doubt this is going to be popular, but in general, I'm against the 
idea of removing the ticket/review process.

Sure, if someone spots a typo, we want to make it easy to fix that.  But 
I don't think typos are really what we're talking about here.

I like two things about OpenLayers:
1) The trunk works.
2) We encourage paired programming or at least some discussion with 
other developers to make sure modifications are well thought out.

I think the first is really a consequence of the second.  I rely heavily 
on a working trunk.  I don't look forward to working on a project where 
this is a liability.

More importantly, I really (really) like the collaborative effort on 
modifications to the trunk.  Sandboxes are a fun and easy way for a 
single developer (or a group of developers) to take off in their own 
direction with an idea.  I like that bringing those modification in to 
the trunk takes time.  More significantly, I like that it also takes the 
eyes of other core developers.

I know there is a wide range between fixing typos and merging major 
sandbox features.  However, with the exception of very trivial typo 
fixes, I think all modifications benefit from more than one set of eyes.

I'm entirely in favor of making it easier to review and commit changes. 
  However, I am against changing our ticket/review policy because I'm 
not interested in working on a project that is really just a bunch of 
independent minded developers slapping together things that look right 
to them.

I look forward to working out a policy that makes everyone happy - and 
I'm confident that we can.  (I also look forward to having Erik weigh in 
on this before we make any changes.)

Tim

Christopher Schmidt wrote:
> I'm beginning to think I was a bit overzealous in stating that all
> patches should require an external review. I've been watching OpenLayers
> development lately, and although I agree that in principle, code review
> is a good thing, I think that it has seriously hamstrung the project
> over the past couple months, due to lack of development time from a few
> core committers who were able to help move things forward in the past.
> 
> Getting more core committers is one helpful step in the right direction,
> but it doesn't completely solve the problem: we're still blocking on
> very simple (one line) patches due to lack of review.
> 
> I'd like to float the idea of allowing commits to trunk without review
> at the discrestion of the patch author. We've got really great
> committers, and I think we have the ability to decide when things
> should/shouldn't go in without a review on every patch.
> 
> That said, when API functionality is involved, I think more thna one
> person should be able to comment before it goes into trunk. So, I think
> that any patch which adds new APIMethods or APIProperties should
> probably be opened as a ticket, and comments should be sought on it
> before it goes into trunk. Additionally, we should be more proactive in
> discussing things both in tickets nd on the mailing lists when there is
> a suggestion in API change.
> 
> How do other people feel about this? I don't want to continue to see one
> line patches left open for 6 months due to lack of review.
> 
> Regards,