Submitting a patch, getting it reviewed, and committed to the Mozilla source tree involves several steps. This article explains how.
The process of submission is illustrated by the following diagram, and each step is detailed below:
Every change to the code is tracked by a bug report in bugzilla.mozilla.org. Without a bug, code will not be reviewed, and without review, code will not be accepted. To avoid duplication, search for an existing bug about your change, and only if none exists, file a new one. Most communication about code changes take place in the bug report, so be sure the bug describes the exact problem being solved.
Please verify the bug is for the correct product and component. For more infomation, ask questions on the newsgroups, or on the #developers IRC channel.
The person working on a bug should be the 'assignee' of that bug in Bugzilla. If somebody else is currently the assignee of a bug, email this person to coordinate changes. If the bug is unassigned, leave a message in the bug's comments, stating that you intend working on it, and suggest that someone with bug-editing privileges assign it to you.
Some teams wait for new contributors to attach their first patch before assigning a bug. This makes it available for other contributors, in case the new contributor is unable to level up to patch creation. By expressing interest in a bug comment, someone from that team should guide you through their process.
All code is supervised by a module owner. This person will be responsible for reviewing and accepting the change. Before writing your code, determine the module owner, verifying your proposed change is considered acceptable. They may want to look over any new user interface (UI review), functions (API review), or testcases for the proposed change.
If module ownership is not clear, ask on the newsgroups or on IRC. The revision log for the relevant file might also be helpful. For example, see the change log for
browser/base/content/browser.js, by clicking the "Hg Log" link at the top of DXR, or by running
hg log browser/base/content/browser.js. The corresponding checkin message will contain something like "r=nickname", identifying active code submissions, and potential code reviewers.
Creating a patch
Changes to the Mozilla source code are presented in the form of a patch. A patch is a commit to version control.
Each patch should represent a single complete change, separating distinct changes into multiple individual patches. If your change results in a large, complex patch, seek if it can be broken into smaller, easy to understand patches representing complete steps, applied on top of each other. This makes it easier to review your changes, leading to quicker reviews, and improved confidence in this review outcome.
Note: See How can I generate a patch for more information about creating patches, formatted properly for easy review. Also look at our Reviewer Checklist, for a list of best practices for patch content that reviewers will check for or require.
All changes must be tested. In most cases, an automated test is required for every change to the code.
Ensure that your change has not caused regressions, by running the automated test suite locally, or using the Mozilla try server. Module owners, or developers on IRC may be willing to submit jobs for those currently without try server privileges.
Submitting the patch
Make sure you rebase your patch on top of the latest build before you submit to prevent any merge conflicts.
If you are an existing contributor, you should use MozReview for submitting patches. See the MozReview User Guide for instructions.
If you use Mercurial, installing the bzexport extension is the preferred mechanism for attaching patches to Bugzilla. The easiest way to install bzexport is to run
mach mercurial-setup. Then run
hg bzexport -e to upload a patch to Bugzilla, without having to leave the terminal.
If you aren't using MozReview, or bzexport, attach the patch file to the bug using the Add an attachment link in bugzilla. Select the patch checkbox when attaching the patch, so reviewers and Bugzilla users can easily see it.
Don't be shy in posting partial patches, demonstrating potential approaches, and asking for preliminary feedback. It is easier for others to comment, and offer suggestions, when a question is accompanied by some code.
Thorough code reviews are one of Mozilla's ways of ensuring code quality. Every patch must be reviewed by the module owner of the code, or one of their designated peers. Patches which cross modules, change APIs, or have security-related changes must have an additional super-review.
For more information about the review process, see the code review FAQ. If your change affects the user interface, see Requesting feedback and ui-review for desktop Firefox front-end changes.
To request a review, or super-review, click the 'Details' link for the attachment in Bugzilla. On the left side are dropdowns with various labels. Find the 'review' entry, change the flag to "?", and enter the email address of the module owner, or peer, who will be reviewing the patch. Remember to submit any changes!
Getting attention: If a reviewer doesn't respond within a week, or so, of the review request:
- Join #developers on Mozilla's IRC server, and ask if anyone knows why a review may be delayed. Please link to the bug too.
- If the review is still not addressed, mail the reviewer directly, asking if/when they'll have time to review the patch, or might otherwise be able to review it.
- As a last resort, ask in the appropriate newsgroup on
Addressing review comments
It is unusual for patches to be perfect the first time around. The reviewer may mark 'review-', and list problems that must be addressed before the patch can be accepted. Please remember that requesting revisions is not meant to discourage participation, but rather to encourage the best possible resolution of a bug. Carefully work through the changes that the reviewer recommends, attach a new patch, and request review again.
Sometimes a reviewer will grant conditional review, by marking 'review+', but indicating minor necessary changes, such as spelling, or indentation fixes. All recommended corrections should be made, but a re-review is unnecessary. Make the changes, attach the revised version, and use the relevant checkbox on the attachment page to render the original patch obsolete. If there is any confusion about the revisions, another review should be requested.
Sometimes, after a patch is reviewed, but before it can be committed, someone else makes a conflicting change. If the merge is simple, and non-invasive, post an updated version of the patch. For all non-trivial changes, another review is necessary.
If at any point the review process stalls, for more than two weeks, see the previous 'Getting attention' section.mozilla.support.thunderbird
In many open source projects, developers will accept patches in an unfinished state, finish them, and apply the completed code. In Mozilla's culture, the reviewer will only review and comment on a patch. If a submitter declines to make the revisions, the patch will sit idle, until someone chooses to take it on.
Pushing the patch
A patch can be pushed (aka. 'landed') after it has been properly reviewed.
Note: Be sure to build the application with the patch applied. This ensures it runs as expected, passing automated tests, and/or runs through the try server. In the bug, please also mention you have completed this step.
Submitting untested patches wastes the committer's time, and may burn the release tree. Please save everyone's time and effort by completing all necessary verifications.
Ensure your patch is properly formatted, making it as simple as possible for a committer to check against your patch.
checkin-needed keyword to the bug. If Bugzilla doesn't allow you to add the keyword, ask someone else to add it. Community members with commit access, regularly search for bugs with the checkin-needed keyword, often committing in a day or so. If the patch does not get checked, within a reasonable timeframe, join #developers on irc.mozilla.org, asking someone to check on your behalf. In most circumstances, a link to a passing Try run will be required, in order to verify the patch will not cause any new failures after landing.
If pushing the patch yourself, please follow Committing Rules and Responsibilities.
It is possible your code causes functional or performance regressions. There is a tight policy on performance regressions, in particular. This means your code may be dropped, leaving you to fix and resubmit it. Regressions, ultimately mean the tests you ran before checking in are not comprehensive enough. A resubmitted patch, or a patch to fix the regression, should be accompanied by appropriate tests.
After authoring a few patches, consider getting commit access to Mozilla source code.