Caution: long!
On 10/03/2011 09:30 AM, Yaron Koren wrote:
Hi,
I think developer accounts on the Wikimedia SVN repository should be easier to get.
I agree.
I say this because a consultant of ours at WikiWorks, Ike Hecht, asked for a developer account last week and was rejected. He created his first major MediaWiki extension, Ad Manager, recently, which I added to the repository a few weeks ago - you can see it here:
http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/AdManager/
When he requested access, this was the relevant part of the response from Sumana:
"Right now, we are not approving your request for commit access. I'm sorry. We'd like for you to get more practice writing code for MediaWiki, submit patches for review via Bugzilla attachments, and ask us for comments... Please come back and request access again in a few months."
I checked with Ike to ensure that he was okay with us discussing his rejection in public -- he hadn't known, but said it was okay. So I'm pasting here in full the response I wrote to Isaac, after having Rob, Chad, Tim, and Aaron review his request and code:
Thank you for requesting commit access, and my apologies on the delay in response. (Developers have had their attention taken by issues around the deployment of MediaWiki 1.18 on a bunch of wikis.)
Right now, we are not approving your request for commit access. I'm sorry. We'd like for you to get more practice writing code for MediaWiki, submit patches for review via Bugzilla attachments, and ask us for comments. Please read
http://www.mediawiki.org/wiki/Manual:Coding_conventions http://www.mediawiki.org/wiki/Security_for_developers
Please come back and request access again in a few months. Thank you for being part of the MediaWiki community, and please feel free to find us in the #mediawiki channel on FreeNode IRC for more detailed feedback.
The bits that were left out of the excerpt, but that I do think are relevant: I pointed him to further reading, to help him improve his code quality, and suggested that he talk with us on IRC for more detailed feedback, again to help improve his code quality. In general, this is the kind of rejection notice I send, when Chad, Tim, and Aaron decide to reject an applicant. If an applicant has SQL injection or cross-site scripting issues in their code sample, then I specifically advise them about that using a templated text.
I would like to provide more constructive and specific feedback, but since that would demand more time per request and we're usually backlogged, I've chosen speed of response over length, and suggested to rejected applicants that they ask for more feedback if they want it. About a third do.
I don't know whether this is WMF policy now, or a personal decision from Sumana, or a decision made by someone else
As others in this thread pointed out before I got to it -- I'm not the one making the decisions on whether to grant access. Right now it's Chad, Tim, and Aaron.
but in any case I don't understand it. It seems to me that there are two valid reasons for not simply allowing everyone to get a developer account: the first, and major, reason is to prevent malicious users from vandalizing or deleting code. The second is to prevent well-intentioned but incompetent developers from checking in buggy and/or badly-written code that requires lots of fixes and review time by the reviewers. In both cases, the person's presence in SVN would cause more harm than good.
That second point sometimes doesn't get enough attention: yes, when we grant commit access, we are committing to keeping up with post-commit review of that person's code. If the person looks likely to make a lot of mistakes, that adds to our load pretty substantially. And so reviewers get a bit fearful about saying yes. (This of course ties into our ongoing exhortations to code reviewers to be revert-happy, but that's a little beyond the scope of this email.)
Neither of those cases apply here - the Ad Manager code was well-written, and it works. If you're curious, you can see for yourself the kinds of fixes and changes that were made to the code after it was checked in - all minor stuff, the only major thing being that the extension originally included support for MediaWiki 1.15, which people thought was unnecessary.
I believe the reviewers' opinion of this code differed markedly from Yaron's, which is the reason for the rejection.
Clearly a higher bar is being applied here than what's spelled out in the mediawiki.org documentation - which only says that "we don't have time to train programmers from scratch":
Yes, and that's wrong. Once we straighten out what the actual criteria should be (and I suspect they'll include a division between core and extensions access), I'll want to update that page.
Note, by the way, that if there's a more stringent policy in place now, it's not being applied consistently, because the students in this year's Google Summer of Code got developer access after much less proof of programming ability.
Yes, there are groups of developers whom we don't screen hard at the moment of commit access request: Google Summer of Code students, for example, and Wikimedia Foundation developers. The commonalities: they're working as part of teams, they're mentored/managed by developers with commit access, they've already been through at least some competence gating process run by people we trust, and they've made dedicated formal commitments to the work.
I feel somewhat comfortable with this, because we're dealing equally with anyone who's in that situation, paid or more volunteer-ish.
It seems to me that if someone writes an extension that basically matches the MediaWiki guidelines, works, and does something useful, they should pretty much be granted automatic access to an account, because they will have proved that their presence will be a net positive overall. Any thoughts on this?
The wiggle words in there are "basically" and "works". As I see it, the commit access reviewers are very concerned about the code quality of incoming code, because of the post-commit review issue.
And out of curiosity - is there a new policy in place?
I think the current situation is not reflective of our stated policy, and that we should both change the situation and articulate a clear policy. My proposal:
* Continue to be strict with commit access requests for core, but be far more lenient with commit access requests for extensions * Get more eyes on the commit access review queue, especially for extensions, to ensure reviewers have the time to give applicants specific constructive feedback * Continue to use OTRS for now, because it's a proper ticketing system, and because it allows some privacy for rejection, because I think that's apt to allow more people to apply, and allow us to give more specific and critical feedback to novices in a way that might feel hurtful to them in public; ** split the OTRS queue into core and extensions queues so we can have more reviewers for the extensions queue
I'll give further replies to others' posts in this thread.