User:MarkAHershberger/Weekly reports/2010-W01

(I'm trying to keep the same schedule as Tim Starling right now, so my first week is complete.)

My first week working at Wikimedia had some highs and lows. I'll get to those in a second. First:

= Culture =

I finally made contact with Tim Starling. The first day, he gave me some stuff to read over (Subversion, Security for developers, Coding conventions ), told me to add myself to self to USERINFO and that was about it. That was the academic portion of my introduction to the culture.

The next day, Tim gave me my first task (see the next item). I asked a lot of questions about how people use the CodeReview tool. I'm excited and a little intimidated (and I've a few concerns). First it shows a real concern for code quality. I like that. So many people are making so many commits that someone — Tim and others — has to let people know as soon as possible if what they've done is going to cause problems.

Since people are actively watching the commit queue and (as I found out later) changes are going live on some Wikis almost instantly, there is a good deal of feedback when you screw up. This is great because you discover problems quickly. As I found out this week, I lack some discipline in my commits. That will change.

Its intimidating because that feedback is right there. I was properly chagrined when I was reminded this week “Well, you would have found that if you had done your own code review.”

My concern comes from the lack of testing. There are plenty of parser tests (though more are needed), but the rest of the code is largely untested — or at least — not tested as regularly as the parser. While it's understandable to focus so much on automated testing of the core of Mediawiki, we should have similar resources made available to the entire code base. The first step in this process should be choosing a testing framework: TAP or PHPUnit. I'd recommend PHPUnit.

A final note about the culture surrounding the MediaWiki code base. In the past, I've seen open source project that discourage white space commits or non-functional, “noisy” commits. Mediawiki appears to be influenced by the attitude that the Wikipedia encourages. People like to make their mark on the code and they'll do things like changing the order of expressions in a test or standardizing the capitalization of keywords (e.g.  vs  ). Some of these changes bug me, but I'll have to get used to it. After all, it means I can unleash my own OCD if need be.

=  =

Tim asked me to work on the  code on Tuesday. He described some of the convoluted logic and how it was causing problems.

First, he said, clean up the code. Get it to match the coding conventions. Doing this gave me a good chance to familiarize myself with the code and to make some small commits without being too dangerous.

Next he had me refactor the  method. This was moderately more difficult and I used the opportunity to get the phpUnit tests working again. I ended up adding a number of tests for the method itself.

Finally, he asked me to refactor the  class so that it wouldn't have such a weird integration with the   class.

After poking at it a bit, I decided the best way to deal with this would be to rip out all the code referring to titles and rewrite it. I chose this code as the target of my wrath because the changes that  was making to the titles caused it to have some weird and convoluted dependencies on the   class.

I was a bit too aggressive, though. Instead of checking the code carefully to determine if it was part of the problem before removing it, I just cut out anything with “title” in it.

A day and a half later, I'd finished my work and checked it in. Unfortunately I didn't do a thorough review of my code. Nor did I write any really good tests for the changes I made. Still, after my initial commit I did find several problems by running the  suite against my code.

At this point, I still had a few hours left in the day and no one was reporting problems with my code (yet) so I decided to go look at bugzilla (see below). In retrospect, I probably should have spent some more time reviewing my code and testing the changes I made.

On Friday, I saw that there were some complaints about the changes I had made to the LanguageConverter class. I set to work fixing them and a few other problems. I ended up adding the ability to check Titles in the parserTest suite so that I could have checks in place in the future.

= BiDi issues =

When I looked at bugzilla, I searched for high priority, high severity bugs (even though the linked search displays normal priority bugs) in the LanguageConverter and Internationalization sections of MediaWiki. The one that really stood out to me was Bug 6100 which, despite being marked “High” priority and “Major” severity hadn't been worked on for quite a while.

After testing it out on a couple of wikis and my own testing installation, I figured that most of the work was done, but that one really annoying part of the bug — the direction of test when the User's language was different than the Content language — could be fixed pretty easily.

After making my commits, I left for the day.

I came back on Friday to a few complaints. I wasn't aware that I had been running without  and that TranslateWiki could apparently run off of trunk with   enabled. I spent a few minutes fixing the bugs and documenting why the changes I made were necessary before I returned to my work on the.

= Summary =

I've had a lot of fun and enjoyed the refactoring challenge, but my ego didn't get too big: I was slapped down because of my boneheaded mistakes.

= Raw Notes =

03

 * made contact with User:Tim_Starling
 * Read over items he gave me
 * http://www.mediawiki.org/wiki/Subversion
 * http://www.mediawiki.org/wiki/Security_for_developers
 * http://www.mediawiki.org/wiki/Manual:Coding_conventions
 * WM Operations
 * Added self to USERINFO
 * Started user page.

04

 * Cleaned up LanguageConverter.php
 * Made tests/* work with phpunit
 * Documented $wgVariantArticlePath

05

 * Refactored getPreferredVariant to hopefully reduce bugs
 * Produced a number of tests that should point out future problems
 * Started work on cleaning up title transliteration

06

 * Refactored how LanguageConverter and Parser work together to translate (or not) titles. Pulled callbacks to Parser out of LanguageConverter.  Removed some checks for MagicWords from LanguageConverter
 * Did some work on RTL/LTR for when wgLang and wgContLang go two different directions. SpecialPages now work better, too.

07

 * In my LanguageConverter refactoring yesterday, I was too aggressive and removed the -{T| }- rule that allows the title to be set differently based on the preferred language variant. Fixed.
 * My LTR/RTL work caused some problems. Because I didn't comment my changes clearly, IAlex attempted to fix the problem incorrectly.  I reverted his changes, fixed the underlying problem, and added a nice long comment about what was going on.
 * I updated the parserTests so that test could be written that would change the title (see the first item for today) and found what I think are some bugs in the LanguageConverter code. I added disabled tests so that I can research them later.
 * Discovered that I wasn't using E_STRICT, but should be. In trying to set up my environment, I was able to get php-cli to run with E_STRICT, but still haven't managed to get mod_php to do it.
 * Discovered that TranslateWiki seems to run off of svn HEAD.
 * Went and peaked at other BiDi bugs.