[OpenLayers-Dev] Commits without review (Was: #1218:
Serialization doesn't wrap nodes in documents)
tschaub at openplans.org
Tue Dec 18 17:13:30 EST 2007
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.
Christopher Schmidt wrote:
> On Tue, Dec 18, 2007 at 07:16:15PM -0000, OpenLayers wrote:
>> #1218: Serialization doesn't wrap nodes in documents
>> I like that this issue got a ticket. And the review wasn't that painful
>> was it? Did anybody learn anything? Perhaps nobody cares to.
> The problem is not that this particular review was that difficult -- it
> was easy, because Paul cared about it.
> However, #1082 is a counter-example. It's:
> 1. Shorter
> 2. Less invasive
> 3. Tested.
> Yet it has sat for 2 months without review.
> What would have happened if Paul were not actively working on the same
> problem? This problem has existed in Safari without anyone working on it
> since we first started serializing XML. Had I fixed it on my own, how
> long would it have been before it got reviewed?
> There are now two OpenLayers apps out there that have modified
> their Layer.Google in order to add Hybrid layers as a seperate type --
> a problem solved by #1082. Instead of supporting them, I have to have
> them continue to maintain a custom hack in their library, something that
> I feel it's important to work to prevent.
> If time-to-review-completed was proportional to the difficulty of
> reviewing, we wouldn't be having this conversation. The patches that I'd
> like to accelerate the commit of are so minimal as to be practically
> inconsequential -- and I'm perfectly willing to take the flak if they
> fail unexpectedly, and to offer help to people who need help with them.
> But patches like this that sit for months are not encouraging to anyone
> -- neither myself, nor the people who I was helping out by writing the
> patch to support. And requiring review for patches of this type simply
> takes away review time from the things that actually *need* review --
> like the Projection support, or Style/Rule support. The former is
> outstanding for 3 months now with relatively little change.
> We have limited developer bandwidth. Instead of wasting it on things
> like #1082, I'd rather have effort continue on things like #1037, which
> I would love to have someone else try.
> Note that the lack of reprojection support has caused:
> * Citysafe
> * Ordnance Survey
> * OpenStreetMap
> to all write their own code for reprojection. Citysafe had an issue with
> their reprojection code -- exactly the same issue that
> http://trac.openlayers.org/ticket/1037#comment:3 addresses. The Ordnance
> Survey code has this bug as well (but it's less prominent due to the way
> they do reprojection). OpenStreetMap depends on the support encoded into
> #1036 -- and four different apps have reimplemented the equivalant of
> the MousePosition and Permalink/ArgParser changes, with the inherent
> bugs that reimplementing the same things over and over cause.
> So, my goal is to make it so that the little things like #1082 get out
> of the way so that the big things -- like #1036 and #1037 -- can be
> reviewed in a more timely manner, because they're not drowned out by
> largely unimportant noise.
> My suggestion in the earlier email thread was "Open a ticket for
> anything you want to be in a statement about the release." "Safari
> support for XML Serialization not including namespaces" is something
> that I think makes perfect sense to have in the changelog about a
> release, so it should get a ticket. I just don't know that I understand
> what anyone gained from the process of Paul looking at the code.
More information about the Dev