Submitting a patch and 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 the steps are detailed in the sections below it.
Preparation
Every change to the code is tracked by a bug 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 the change, and if none exists, file a new one. Most of the communication about code changes takes place in the bug, so be sure that the bug describes the exact problem being solved.
Please verify that the bug is in the correct product and component. For more infomation, ask questions on the newsgroups or on the #developers IRC channel.
The person who is working on a bug should be the "assignee" of that bug in bugzilla. If somebody else is currently the assignee of a bug, email that person to coordinate changes; if not, leave a message in the bug stating that you are 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, so that it remains available for other contributors in case a patch is never created. By expressing interest in a bug comment, someone from that team should guide you through the process they are using.
Module ownership
All code is supervised by a module owner. This person will be responsible for reviewing and accepting the change. Before writing code, determine the module owner and verify with them that the proposed change is 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 can 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 potential reviewers for the code in question.
Creating a patch
Changes to the Mozilla source code are presented in the form of a patch. A patch is essentially a commit to version control.
Each patch should represent a single complete change, so you should separate distinct changes into multiple patches. If your change results in a large, complex patch, then see whether it can be broken down into smaller, easy to understand patches representing complete steps applied on top of each other. This will make it easier to review your changes, leading to quicker reviews and better confidence in the review.
See How can I generate a patch for more information about creating patches that are formatted properly for easy review. See also our Reviewer Checklist for a list of best practices for patch content that reviewers will check for or require.
Testing
All changes must be tested. In most cases, an automated test is required for every change to the code. In some cases where it is not possible to write an automated test, manual tests called Litmus tests are used.
Ensure that the change has not caused regressions by running the automated test suite, locally, or using the Mozilla try server. A module owner or developer on IRC may be willing to submit jobs for those 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
. After that, simply 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. Check the patch box when attaching the patch so that reviewers and bugzilla users can read it.
Don't be afraid to post partial patches demonstrating potential approaches and ask for preliminary feedback on them. It is easier for others to comment and offer suggestions when a question is accompanied by code.
Getting reviews
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 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 and change the flag to "?", and enter the email address of the module owner or peer who will be reviewing the patch. Remember to submit 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 in question, as well).
- If the review is still not addressed, mail the reviewer directly, asking if/when he'll have time to review the patch or who else might be able to review it.
- As a last resort, ask in the appropriate newsgroup on
news.mozilla.org
.
Addressing review comments
It is unusual for patches to be perfect the first time. 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 changes that must be made, 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, but for all non-trivial changes, another review is necessary.
If at any point the review process stalls for more than two weeks, see the "Getting attention" section above.
In many open source projects, developers will accept patches in an unfinished state, finish them and apply them. In Mozilla's culture, the reviewer will only review and comment on a patch. If the submitter declines to make the revisions, the patch will sit unless and until someone else chooses to take up the effort.
Pushing the patch
A patch can be pushed (aka. landed) after it has been properly reviewed.
Please build the application with the patch applied, ensuring it runs and passes automatic tests (and mention you have done so in the bug) and/or push to the try server (also stating this in the bug). Submitting an untested patch wastes the committer's time and may burn the tree. Please save everyone's time and effort by completing all necessary verifications.
Please ensure that the patch is properly formatted to make it as easy as possible for a committer to check in your patch.
Add the checkin-needed
keyword to the bug (or 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 and commit within a day or so. If the patch does not get checked in within a reasonable timeframe, join #developers on irc.mozilla.org and ask someone to check it on your behalf. In most circumstances, a link to a passing Try run will be required in order to verify that the patch will not cause any new failures after landing.
If you're pushing yourself, please remember to follow Committing Rules and Responsibilities.
Regressions
It could be that your code causes functional or performance regressions. There is a tight policy on performance regressions in particular, which means your code may well end up being backed out and you will have to fix it and resubmit it. Regressions mean the tests (which you ran before checking in, didn't you) are not comprehensive enough, so 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.