Mozilla Development Strategies

This article offers some strategies and suggestions to help developers stay productive.

Work on the most important bugs first

Everyone likes to check in. But keep in mind that your checkin rate isn't everything. It's better to fix one data loss bug, crasher, or performance bug that really affects the user than edge case bugs that are rarely seen or minor bugs.

Make sure documentation gets updated

If the bug you're fixing is likely to require an update to developer documentation once it's fixed, be sure to add the dev-doc-needed keyword to the bug (or ask someone to do it, if you don't have editbugs privileges on Bugzilla). This puts the bug on the radar of our documentation team to ensure that once the bug is resolved, the documentation will be updated appropriately. If you don't mark the bug, your work might go unnoticed by the docs team!

Of course, our documentation is a wiki; you can really help by updating the documentation yourself. Even if you're not comfortable with your writing skills, keep in mind that our helpful, happy documentation gnomes will follow along behind you and clean up for you.

Take extra time to do it right the first time

It's also better to come up with one really solid, well tested, well commented, clean, easy to maintain piece of code than a bunch of quick fixes. You (or someone else) will be back in that code someday. It's easier to do it right once, when your mind is in the problem, than to do it once quickly and then have to come back and do it again the right way. The only thing worse than not understanding someone else's code is not understanding your own.

Test your code

QA's job is to assure quality, not to add quality. That's your job. It's your responsibility to find and fix the problems in your code before checking in. When you land your bug-free code, it's QA's job to assure others that it's really bug free.

Make sure you appreciate anyone who finds a bug in your code. Appreciate it even more if they find it before you ship. They're giving you a second chance to get it right before it becomes a bug that affects users.

Minimize how you are affected by regressions

Unless you are the type to go out and tackle daily regressions, you need to find a way to not let them affect you. You don't want to have one tree, update it every morning, and then be stuck waiting for blockers to clear.

Have multiple trees. At least one of them should be updated every day. You'll need this one for regressions that require your immediate attention (a new crasher, a blocker, or problem in your area, for example). Use this same tree for small, quick bugs or recent regressions or crashers. For your other tree, don't update as often. Update when you know that the tree is in good shape. Do your primary development in this tree. You'll need to update, rebuild, re-test and make a diff against the current trunk once you feel your fix is done. But, daily regressions won't block your primary development.

As far as keeping trees up to date goes, the longer you go without updating your tree, the more likely you are to have CVS conflicts. If you update every day for a week, you might not have any conflicts, but if you update once a week and you've changed a lot of code, you'll most likely have conflicts.

See mozilla\config\fast-update.pl for a fast way to update your tree. You can update your main mozilla tree (minus nsprpub, etc) in 1-2 minutes.

Work on multiple bugs in parallel

You should be working on the most important bugs first. But those might be difficult bugs (hard to reproduce crashers, big rewrites for performance, etc.) which can take several days or weeks to complete, plus the time for reviews. In your other trees, work on smaller, easier bugs. Pick bugs that won't cause conflicts with the work you're doing in your primary tree. Pick bugs that should only take you a few hours or a day to fix. In theory, fixes for these bugs will be quick to review.

If you have problems finding smaller, easier bugs to work on, start by looking for crashers. Maybe the problem can be turned into an assert, or fixed altogether. You can use talkback queries or Bugzilla queries to find crashers.

Look for problems in existing code. Find and fix string usage problems. Find and fix bad XPCOM macro usage. Switch some code over to nsCOMPtrs. Look for XXX or TODO in the code, verify the code still needs fixing, and fix it.

Smaller patches get reviewed faster

If you find that you spend a lot of time waiting for reviews, keep in mind that patch size is not linear to time to review. A twenty line patch doesn't take twice as long to review as a ten line patch, it usually takes much more time. If you can divide and conquer what you are working on into smaller chunks, you will find you'll get faster reviews. Of course, not everything can be divided up. And, shorter fixes aren't necessarily better than longer ones.

Having parallel trees should allow you to work on other items while you are awaiting reviews.

Get advice before working on a patch or feature, instead of after

If you are blocked, or stumped as to what to work on, talk to a lead sooner rather than later. Most likely they will have a bug they can hand off to you, or they can help get you unblocked. Since they are going to be reviewing your code later, run your plan of attack by them first. It's better to have an idea rejected, than a big patch.

If you are blocked, but have something worth checking in, check in using #ifdef, prefs, or "alternative" files

Sometimes you'll work on something, but you can't land it because something else is blocking you. You can still land (assuming you get reviews) if the code isn't on by default. This also makes it easier to get help on the problem. It's easier to tell someone set a pref, or apply a small patch, or set a #define than to apply a monster patch.

For C++, use #define, #ifdef, #else and #endif

For XUL / JS, and you're doing something major, add your new files to the tree and then have it be a simple jar.mn patch to enable it.

You can also have code that checked in, but controlled with a pref and off by default.

Make sure you have the right fix when you go for reviews

When tempted to do a first pass fix and fix the fallout later, do it right the first time; don't assume you'll be able to file a "fix it" bug and fix stuff you know about later -- chances are the reviewer will make you do the work anyway.

If it takes you five extra minutes to do something the "right" way rather than twenty minutes debating it with the reviewer, you've just saved yourself and the reviewer valuable time. Obviously, when doing it the "right" way means re-engineering large amounts of code, then incremental work is better; it's a fine line.

Don't drag out reviews by fighting the reviewer in Bugzilla (or email)

When getting a review, try not to belabor a point that a reviewer is debating you on. If the debate is involved, solve it in person, on IRC or over AIM. A five minute discussion is worth an hour of e-mails.

Do thorough code reviews

When you review someone else's code, review thoroughly. If another engineer checks in code that causes regressions or introduces bugs, you might be the one who has to fix it later. Save yourself (and others) time later by doing a solid code review.

Review your own code before you ask for reviews

Practice your reviewing skills on your own patch before you seek reviews and a super review. Better that you catch something, than the reviewers or super reviewers. You might find a problem, or find that you left in some dump() or printf() statements, or have messed up the whitespace.

If you're working on something massive, start a branch

If you are working on something big, and you want to be able to check in incrementally without getting reviews, create a branch. But having a branch means dealing with conflicts when you land, and waiting a long time for reviews.

Here's how to create a branch:

From your Linux or Mac OS X box:

# start from your trunk tree on your local disk
cd mozilla
find . -type d \! -name CVS | xargs -l -P10 cvs tag -l MY_BASE_TAG > & ../taglog1.txt
find . -type d \! -name CVS | xargs -l -P10 cvs tag -b -l MY_BRANCH_TAG > & ../taglog2.txt

From your Windows box:

cvs co -r MY_BRANCH_TAG mozilla/client.mak
cd mozilla
edit client.mak, putting MY_BRANCH_TAG in the right places.
cvs commit client.mak
nmake -f client.mak