On Thu, Jan 5, 2012 at 9:02 AM, Rob Lanphier robla@wikimedia.org wrote:
I've been going over what we have, and it may not be all *that* dire, though I'd like to get everything tagged before making that pronouncement.
Here's the tagging we've got so far: http://www.mediawiki.org/wiki/MediaWiki_1.19/Revision_report
There's a couple of big areas that stand out:
- "educationprogram": can we defer these until after we deploy?
- "socialprofile": doesn't block 1.19 deployment. My understanding is
that ashley would still like review on these, though. We can mark these as "nodeploy", though that won't take them out of the graph (only the revision report above)
Yes please. SocialProfile commits aren't as urgent as, say, commits to an extension already deployed or soon to be deployed on WMF wikis, but it'd benefit me as a developer and all the end-users to have those commits reviewed. Like the old saying goes, to err is human, but obviously we'd prefer that any major errors are spotted in code review as opposed to when the code is already live on someone's wiki. ;-)
At the risk of sounding like a broken record, I will state for the record that in my opinion, the "deferred" state on CodeReview should *never* be used for anything. If the fact that some extensions or other stuff what should not be showing up in graphs, stats and whatnot is showing up, then the solution is to make whatever code generates those stats to exclude the stuff you don't want to show up. For example, WMF sites aren't running any Semantic extensions (as far as I'm concerned -- do correct me if I'm wrong!), so it should be rather easy to make stats code to exclude paths matching trunk/extensions/Semantic*.
I've been under the impression that code added to our SVN repository is supposed to serve as an example to other developers: this is good stuff, write your code like this. When something isn't quite up to our standards (as defined in [[mw:Manual:Coding conventions]]), someone will let you know via CR comments. That's how it's supposed to work, but in practise review seems to be too WMF-oriented at times. It's obvious that WMF is an important user of MediaWiki, but it doesn't reduce the importance of things not used by WMF. For example, some third-party user might've built a big wiki on the Semantic stack and then after an upgrade, things break due to a recent code change -- there could be an error on a recent Semantic commit, maybe even something as simple as a typo, that no-one spotted early enough because it's easier to mass-defer commits via CR's mass-status-change tool instead of opening up each and every revision to see what was done.
One of the various extension that I maintain is SocialProfile ( http://www.mediawiki.org/wiki/Extension:SocialProfile). In late 2011, Markus Glaser performed an in-depth review of the extension (see http://www.mediawiki.org/wiki/User:Mglaser/Code_Reviews/Social_Profile); it was the first such in-depth review of "my" extensions, and quite large, too. Kudos to you, Markus! It seems that many of the issues pointed out by Markus in his review could've been caught early on, when the appropriate code was first added to the repository, but back then there was literally nobody besides me interested in SocialProfile development (and, to be fair, CodeReview didn't exist very early on). Now there are a few people who occasionally comment on social commits and point out obvious mistakes -- that's awesome, and it certainly is a positive change from the past, but a few more reviewers couldn't hurt.
To help out in code review instead of just whining on and off-list, I'm offering to review extensions that their developers would like to have reviewed, especially ones not used by WMF wikis (i.e. the ones that usually get the nasty "deferred" status on CR otherwise). Feel free to contact me either via e-mail, IRC, or the on-wiki talk page (see http://www.mediawiki.org/wiki/User:Jack_Phoenix for my contact details).
Thanks and regards, -- Jack Phoenix MediaWiki developer