(This is copied from the first part of last week’s weekly report. I’ve copied it here since more people may be interested in what it feels like to work in a highly visible open source project.)
I’m halfway through a 3 month contract (a relationship with the WikiMedia Foundation that I hope to continue) and it seems like a good place to write up some of my lessons learned. Too many times I haven’t spent enough time going over my code. Or, when I’m refactoring someone else’s code, I don’t look over it enough. Sometimes this is due to my own inexperience with the MW code base: If I had more experience, I would have a better idea from just looking at the code that dieUsage() was being used incorrectly. Still, experience can be created, and when it is experience you lack, the effort to create experience must be made. This is where testing comes in. More than once while refactoring the UploadChunks api, Tim has just looked at the code and told me I wasn’t testing it properly. The same could be said for more trivial things like whitespace. Mixing whitespace commits with more substantive changes as well as just the way Emacs formats the code by default have irritated other developers. From this experience, I think I need to borrow an idea from Atul_Gawande’s Checklist Manifesto and make — gasp — a checklist. The checklist is the first step, but the second would be automating as much of the checklist as possible and creating a pre-commit hook that I could use to check my own code. From my experience, here are some ideas for a checklist:
- Whitespace use.
- Tabs, not spaces
- avoiding spaces for indention and lining up code
- spaces around parens
- Code correctness.
- Ensure each exit point has a test. (Since I prefer to write unit tests for my code, noting a test for each exit point in a code comment would take care of this. An automated check would just verify that the exit point has a notation.)
- Make sure no E_STRICT warnings are thrown during tests
Note that any automation here is going to fail to really enforce anything: it is impossible to verify code-correctness completely without actually running the code. Instead the idea is not to verify that the code is bug-free, but just to make sure that I’ve caught the things that more experienced MW developers aren’t going to roll their eyes at. That said, the number of developers actively working on MW code as well as the CodeReview extension on mediawiki.org make developing MW code an overwhelmingly positive experience. These things mean that — combined with the depth of PHP and MediaWiki knowledge and care that people take — problems see the light of day really quickly. When your code is likely to end up on one of the top-10 sites on the Internet, this amount of care is crucial.