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 and the associated code review, so be sure the bug describes the exact problem being solved.
Please verify the bug is for the correct product and component. For more information, 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. Firefox and related code is stored in our Mercurial server. We have extensive documentation on using Mercurial in our guide, Mercurial for Mozillians.
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.
Also ensure that your commit message is formatted appropriately. A simple commit message should look like this:
Bug 123456 - Change this thing to work better by doing something. r=reviewers
r=reviewers part is optional; if you are using Phabricator, Lando will add it automatically based on who actually granted review, and in any case the person who does the final check-in of the patch will make sure it's added.
The text of the message should be what you did to fix the bug, not a description of what the bug was. If it is not obvious why this change is appropriate, then explain why in the commit message. If this does not fit on one line, then leave a blank line and add further lines for more detail and/or reasoning.
You can edit the message of the current commit at any time using
hg commit --amend or
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.
Mozilla uses Phabricator for code review. See the Mozilla Phabricator User Guide for instructions.
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.
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, you will need to specify one or more usernames either when you submit the patch, or afterward in the UI. See the Mozilla Phabricator User Guide for details.
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 use the ‘Request Changes’ action 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 with the ‘Accept Revision’ action but will also indicate minor necessary changes, such as spelling, or indentation fixes. All recommended corrections should be made, but a re-review is unnecessary. Make the changes and submit a new patch. 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.
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.
Add the tag "check-in needed" on the revision(s) in phabricator.To do so, click on the "Edit" button on a phabricator revision, and start typing "check-in needed" in the Tags field. It should auto-complete. If Phabricator doesn't allow you to add the keyword, ask someone else to add it. Community members with commit access, regularly search for revisions 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.
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.