Getting your patch in the tree
From MDC
Once you've made a change to the tree, tested it, and created a patch, you can contribute it back to the main source tree. This process consists of three main steps:
- Attaching the patch to the relevant bug in Bugzilla
- Getting it reviewed, and finally
- Getting it checked into the tree
These steps are described in detail in the rest of this article.
[edit] Attaching the patch
The first thing you should do to get people looking at your patch is attach it to the relevant bug in Bugzilla. You should first look for an existing bug covering the issue your patch solves and, if you can't find anything, file a new bug. Make sure to make it clear what exact problem you're solving and, if it's not clear from the patch, how you are solving it. The clearer the issue and the fix is, the more likely it will get looked at.
Once you've found or filed the relevant bug, attach the patch you created earlier to the bug using the "Add an attachment" link. Select the "patch" checkbox so that Bugzilla can show a prettified, side-by-side version of it if someone wants to view one (via a "Diff" link next to the patch). If you have the editbugs bit set on your account (see Preferences, then Permissions; it'll be listed if you have it), feel free to check the "take bug" checkbox while you're at it. You can also edit the attachment's attributes after it has been submitted via the "Details" link in the Attachments table in the bug.
You usually should CC yourself to a bug on which you're working, if you haven't already, by checking the "Add to CC list" checkbox next to the comment area in the bug. Doing so means you'll receive email when changes are made to the bug so you can keep track of its progress toward being fixed.
The next step is figuring out who the appropriate reviewers for the patch are and asking them to review the patch.
[edit] Getting the patch reviewed
Code reviews are Mozilla's way of ensuring code that's not up to snuff doesn't get into the repository. It ensures high code quality: readability, clarity, security, and all that jazz. Currently, most patches have to undergo two reviews: a regular review and a "superreview". Reviewers are specific to given areas ("modules") of the codebase, but any super-reviewer may review any patch (although they may request you choose another if they don't know the code).
Some areas of the codebase have different review requirements. js/src requires only a single review, and toolkit/ and browser/ require only a single review. Tests also only require a single review to ensure their correctness. Feel free to ask in the bug when you request the first review if the code you're modifying has special review requirements.
Module owners and peers review patches for code in a module, and they can help you through the process of getting your patch in the tree. The list of modules and people familiar with them is a good place to find someone to review your patch.
Another good place to look for reviewers is the CVS log for the files you change. For example, if your patch primarily changes browser/base/content/browser.js, you can usually choose someone who's recently reviewed patches to the file by clicking the "CVS Log" link at the top of the MXR page for it. If someone's reviewed a patch, the corresponding checkin message will contain something like "r=foobar" or "sr=baz". "foobar" and "baz" are either email addresses or shortened forms of people's names; entering one in the review field will usually (not always) autocomplete and ask the right person for the review. If it doesn't, visit the bug cited in the checkin comment and find the email address of the reviewer there (usually in a comment with text "r=baz" or similar).
To request review or super-review, click the Details link for the attachment. On the left side are dropdowns with various labels; enter the bugmail address of the person whose review you want in the textbox associated with "review" (analogously for super-review), and change the dropdown from empty to "?". (You can also do this when you submit a patch.) Finally, submit your changes to actually change the flag and request review. You can request both review and superreview at once, and if you feel your patch is suitably small, simple, or obvious, you can make both requests to the same person. (That person may redirect either one to someone else, of course, if he thinks the patch isn't quite so simple.)
Review often isn't a one-time thing; you may need to revise your patch before reviewers are comfortable approving it (by changing the flag to "+"; denying review is indicated by setting the flag to "-"). This is quite common, especially for your first few patches. Just make the changes they suggest, post a new version of the patch, and re-request review, and eventually you'll have a patch that meets their approval.
Sometimes a reviewer will flip the review flag to + but leave a comment at the same time detailing changes he wants made to the patch; if this happens, he wants the changes made but doesn't feel it's necessary for him to re-review them. Just make the changes, post a new version with the changes made (obsoleting the old patch via the relevant checkbox on the attachment page), and set any remaining flags (superreview, etc.) to their values from the previous version.
Something else that sometimes happens is that your patch is reviewed, but before it can be committed, someone else makes a change that conflicts with your patch. If the fix is simple -- use your best judgment -- and non-invasive, just post a new version of the patch; you don't need to re-request review. If the fix is more involved or not particularly obvious to a reader familiar with the code, you should get one last review of it. This is a pain when it happens, but it's not very common and thus is mostly tolerable.
If a reviewer doesn't respond within a week or so of your making the review request (sadly, not entirely uncommon), you have a few options:
- The best option is to jump onto #developers on Mozilla's IRC server and ask about it there. People are most active during weekday afternoons and evenings, PDT, so it's best to ask then if you can.
- You can mail the reviewer directly, asking if/when he'll have time to review the patch or who else might be able to review it.
- You can also ask in the appropriate newsgroup on
news.mozilla.org, although this can be a hit-or-miss proposition. - Finally, you can try another reviewer.
If you have a question about what to do about a particular bug, don't be afraid to post even a partial patch and ask questions. A question from someone with code is treated very differently than a question without code; demonstrating you've put effort into trying to solve a problem will make others much more likely to put effort into it as well.
[edit] Getting the patch checked into the tree
After the attachment for the patch gets an r+sr (or just r+, depending on the module), it's ready for the checkin. If the patch was marked r+ conditional on some simple changes, attach an updated patch, but don't ask for another review. If the person who reviewed the patch trusts you to make the changes correctly, he assumes you can do so and doesn't need to see them for himself. :-)
Please build the application you're changing with your patch applied, make sure it runs, and passes automatic tests.
Submitting an untested patch might waste the committer's time or even burn the tree. If your patches have a bad track record, people might put off checking in your patches, since it takes more time. It also affects your chances to get access to make your own commits.
Add checkin-needed to the Keywords field of the bug; various people regularly run searches to find these bugs, and your patch will probably be committed within a week or two.
If your patch does not get checked in within a reasonable timeframe, try joining #developers on irc.mozilla.org and asking people there to check it in for you.
After you think you've contributed enough patches, feel free to consider Getting commit access to Mozilla source code.