[OpenLayers-Dev] Ticket #1093

[Christopher Schmidt]

On Thu, Jan 29, 2009 at 08:32:17AM +0100, Patrick Valsecchi wrote:
> Hi,
> 
> About this ticket:
> http://trac.openlayers.org/ticket/1093
> 
> I see there are a few missunderstanding. So I'll try to answer to your last 
> comment.
> 
> 1) The quote patch unquote scandal
> 
> I tend to put the two liners inline because that way, I don't have to create a 
> file, attach it to the ticket and you, you don't have to click on the 
> attachment, click on "raw", save the file, go to your shell and apply the 
> patch.

I work on OpenLayers code in a particular way. The way I do it is:
 * Open this page:
   http://trac.openlayers.org/wiki/Release/2.8
 * Go through the list of tickets marked 'review'.
 * Select a ticket
 * Scroll to the attachments section
 * Right click an attachment, click 'copy link'
 * Go to my terminal, type 
    curl '<ctrl-v-to-paste>?format=raw' | patch -p0 
 * Run the new tests, make sure they work
 * svn revert -R lib
 * Run the new tests, make sure they fail.

Any patch that I can do this for, and feel confident solves the problem
without causing other ones, is usually quickly committed. (This has
changed in the 2.8 timeframe, as I am no longer making as much of an
effort to review every patch.)

Any patch that I can not do this for gets bumped below any ticket I can
do this for.

> Did you know that commands comming from the UNIX world tend to expect
> the data to come from STDIN when no file is specified? That way, you
> can just type "patch -p0", press enter, cut&paste the quote patch
> unquote in your shell, press enter and finally CTRL+D.

Sure. And you have pasted only one section of 'working' code into this
ticket, which is an invalid solution, because it would fix MapServer and
break WMS.

> I think it's easier that way for both of us. Don't you?

Using my review process, I would *never see* #1093. It was never marked
for review.

  http://trac.openlayers.org/wiki/CreatingPatches

Is quite clear about the way to get a ticket seen by me. Anyone who
doesn't follow that process is likely to have their patch missed by me.


> 2) The lack of final patch from me
> 
> In your "10/17/07 09:40:58" comment, you said you wanted to see with the 
> MapServer developers first. Since it's a pain in the ass for me to write OL 
> patches, I was waiting for the result of this.

I'm sorry, I never clarified after that. Several days later, I confirmed
the fact that MapServer's extent calculation was broken, and I didn't
update the ticket. This was very much my fault, and I apologize -- it
simply slipped my mind, and the patch never again made it up the review
queue because -- again -- it was never marked for Review, and the only
way I ever look at tickets is if they are marked for review. (If the
review queue is *empty*, I keep looking, but it hasn't been empty since
before 2.6.)

> Why is it so hard for me to write a patch? In the past, all the OL patch I 
> submitted where either rejected or took ages to be committed. Since I need 
> the correction fast to be able to move forward in my job, I have to first 
> change the buggy methods using javascript overriding. That way i can do the 
> work I'm paid for. Then, I write a ticket, modify the OL code to create a 
> patch. After that, i have to go through the pain to argue why I've used 
> solution A instead of B, apply some unwroten coding convention to my patch (a 
> missing space here and there) and, maybe, after a few patch versions and a 
> couple of month of arguing, the modification appears on SVN. Then, I have to 
> remove my "JS override" patch from my code.

You have, at this point, access to at least three committers on the
OpenLayers project within your company. This ticket was only ever
replied to by me. No one else even bothered to offer an opinion. In this
particular case, there was a social mistake I made -- over a year ago --
which may have negatively affected thigns, and I apologize again for
that. I did hold up the process on my end, and it's perfectly reasonable
to blame me for that, and that *may* be why no one else has commented.

However, with those resources at hand, I'll notice that myself -- anad
Erik, acting as release triage team -- are the only ones who have ever
touched this ticket. To me, all this indicates is a lack of free time
on the part of developers to review the problem.

Clearly, review of problems is neccesary, because the patch you attached 
was *broken*. The change was the wong one to make, and no further change
was obviously suggested.

Because the issue is minor and affects few people, this is not a high
developer priority -- as you said, using MapServer layers in general is
not recomended, so it doesn't get attention.

But when I'm the only person commenting, what this reads like to me is a
direct commentary that there are not sufficient resources to look at
every ticket in trac in a timely manner and deal with them.

Since Camptocamp *has* resources available devoted to the OpenLayers
project, but did not use them to look at this ticket, the only thing I
can take from that is that the fix is not important to Camptocamp. I'll
be straightforward here: If an app which is being developed needs a fix,
and the company developing it has resources to commit a fix itself, I
will generally not make it my *personal* priority to make it easier for
the company to continue in their project. There is *no reason* that
one of your coworkers could not have reviewed this patch.

This applies for other aspectso f the project as well. For example, the
MapGuide support is primarily developed by DM Solutions, primarily by
Mike Adair. My personal motivation to work on this is low -- the primary
user, DM Solutions, has access to at least two competent Javascript
coders who can write patches to the code (Zac and Mike), and one who can
review them and commit them (Paul Spencer). Since my knowledge of
MapGuide is low, I leave MapGuide related tickets to be handled by the
people who know them -- primarily DM Solutions. As a result, these
patches tend to linger, because DM doesn't have the resources to devote
to them. Since it doesn't affect users that I interact with, this
doesn't move up my priority list.

I'll note that your original responses to this ticket took place within
*hours* -- much better turnaround time than I've gotten from any other
client. In fact, I have two minor patches to Webkit which have been
outstanding for two years, and regularly find that tickets I write in
other Open Source projects are left untouched for long periods of time.

OpenLayers isn't perfect. We are extremely developer-time limited, and
do not have sufficient resources to attack everything we get -- not even
everything with patches. There are many patches which I hae seen
languish which are much more useful than a minor change to something
that no one uses.

My priorities are two fold:
 1. Corporate. Typically high priority, but limited in scope. These are 
    the things I need to fix for MetaCarta. This happens approximately 
    once every 6 months.
 2. Personal. I approach problems that interest me, and I think will
    have significant benefit to the project. This is the lion's share --
    probably about 95% of my time on OpenLayers over the past year.

It is my belief that there is no other trunk contributor who, at this
point in time, spends much of the latter time on OpenLayers. As a
result, the concentration is on things that affect the work they are
doing on the projects they're paid to work on. 

This is how things happen. There's nothing to do about that -- money
talks. But if you really have a need within your company to fix
something, then drop an email to someone within your company, and get
them to support the work. Even an additional comment of "This is
affecting me and I can confirm that solution $x works without breaking
anything" would completely change the game as far as getting my
*personal* attention goes, because I devote my personal attention to
things that I see affecting people.

Some recent examples: 

  http://trac.openlayers.org/ticket/1908 convenience method for
  geometry.fromWKT.
    I need this pretty much every time I experiement with vectors. 
    Patch uploaded with tests, reviewed and marked commimt within 5
    hours, committed within 6.

 http://trac.openlayers.org/ticket/1822  Vector getDataExtent modiies
   feature
   Wrote testss, committed within 2 days

 http://trac.openlayers.org/ticket/1872 minor improvement to
 getMousePosition
   committed within 1 day

Some of these are me, some of these are elemoine, but all of them have a
significant difference: they followed the standard pipeline, and they
were things we cared about.

If you want something to get done, *make a developer care*. You can do
this by paying them, you can do this by making them interested -- but
you can't do this by uploading incorrect patches, not following up, not
following process -- and then getting upset that they don't review your
patches.

I have seen many good contributors get patches approved in minutes or
hours -- from Camptocamp, from the OpenStreetMap project, from OpenGeo,
from independant contractors working on the project. I have seen many
fine patches languish, even after following the procedure, and uploading
a perfectly reasonable patch with tests and supporting evidence that the
code is correct.

However, when these are not the case -- when a patch doesn't start with
sufficient supporting evidence, when it is in a hard to understand grey
area, when it is not marked for review, when it doesnt have tests... 
all of these cause it to get lower in my personal todo list. I expect
other developers are much the same.

> Why is this process so long? I'll let you answer to that one. I've submitted 
> patches to a lot of other OSS projects and OL is the most painful.

In this case, the process was long because no one has ever created a
patch to the code which fixes the problem and uploaded it to trac --
even as a comment. Not to mention that I expect that this patch would
break a number of tests -- which is not addressed in this ticket at all.

In general, the process is short when your code is succinct and
well-explained. It is also short when it has tests, since that lets us
confirmr quickly and easily that the behavior is correct in the future.
It is also short when a developer -- or even another user -- is
interested in it, because then a second pair of eyes can be attached to
the code, and give it a htumbs up or thumbs down.

In this case, you have none of these. At least one of them is
potentially more within your control than mine -- you have social
relationships of some sort with at least some number of other developers
and users of the code, who could comment and say that they'd like to see
the fix.

OpenLayers is all about scratching developer itches. My itches come from
helping as many users as I can. Your patch didn't scratch that itch --
or any other itch of mine. That's unfortunate. To get patches approved,
you really have to scratch an itch -- or fight hard to prove that it
will become an itch if not scratched. 

Best Regards,
-- 
Christopher Schmidt
MetaCarta