Several of us took off the past few days. But now its time to get back on the Code Review wagon.
CRStats (http://toolserver.org/~robla/crstats/) shows that on December 24th we were had around 500 revisions left for review.
This wasn't too bad except that we CRStats shows we were supposed to be closer to 300 revisions left.
Today is worse. We're up to over 600 revisions for review while Robla's projections show that we're supposed to be closer to 200 revisions for review.
So, I'm gonna pile on with Chad and Brion here and ask you guys to stop working on features and refactoring by Friday (if not today) and help out with reviewing code or, if that isn't your thing, help fix code marked FIXME:
https://www.mediawiki.org/wiki/Special:Code/MediaWiki/status/fixme
We've got about 40 revisions marked FIXME and I'm sure that more will show up as we go through the review queue.
For what its worth, until we branch, I'm going to be putting trunk up on for testing on http://deployment.wmflabs.org/ and asking different users to try it out.
If you have any other ideas for how we can get more eyes on the code we've produced over the past few months, I'm all ears!
Mark.
Thanks for sending out this reminder Mark! Comments inline:
On Wed, Jan 4, 2012 at 5:23 PM, Mark A. Hershberger mhershberger@wikimedia.org wrote:
CRStats (http://toolserver.org/~robla/crstats/) shows that on December 24th we were had around 500 revisions left for review.
This wasn't too bad except that we CRStats shows we were supposed to be closer to 300 revisions left.
Today is worse. We're up to over 600 revisions for review while Robla's projections show that we're supposed to be closer to 200 revisions for review.
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)
Also, as an aside, I've added a couple of psuedo-tags to the graph: http://toolserver.org/~robla/crstats/
The "newly new" psuedo-state is a plot of new revisions added in a given week. The "reviewed" psuedo-state is all revisions that transition from "new" to some other state in a given week.
This shows that the review load has been steadily rising over the past year, which means even though we've been adding capacity, that's not enough to keep up with out either slowing down a little bit to catch up, or really stepping up.
Rob
Rob
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
wikitech-l@lists.wikimedia.org