Compare Revisions

How to Submit a Patch

Change Revisions

Revision 549943:

Revision 549943 by Karlt on

Revision 549969:

Revision 549969 by trevorh on

Title:
How to Submit a Patch
How to Submit a Patch
Slug:
Developer_Guide/How_to_Submit_a_Patch
Developer_Guide/How_to_Submit_a_Patch
Tags:
"Developing Mozilla", "Bugzilla"
"Developing Mozilla", "Bugzilla"
Content:

Revision 549943
Revision 549969
n29      All code is supervised by a <a class="internal" href="/en/Mn29      All code is supervised by a <a class="internal" href="/en-U
>ozilla_Modules_and_Module_Ownership" title="en/Mozilla Modules an>S/docs/Mozilla_Modules_and_Module_Ownership" title="/en-US/docs/M
>d Module Ownership">module owner</a>. This person will be respons>ozilla Modules and Module Ownership">module owner</a>. This perso
>ible for reviewing and accepting the change. Before writing code,>n will be responsible for reviewing and accepting the change. Bef
> determine the module owner and verify with them that the propose>ore writing code, determine the module owner and verify with them
>d change is acceptable. They may want to look over any new user i> that the proposed change is acceptable. They may want to look ov
>nterface (UI review), functions (API review), or testcases for th>er any new user interface (UI review), functions (API review), or
>e proposed change.> testcases for the proposed change.
n38      Changes to the Mozilla source code are presented in the forn38      Changes to the Mozilla source code are presented in the for
>m of a patch. See "<a href="/en-US/docs/Mercurial_FAQ#How_can_I_g>m of a patch. See "<a href="/en-US/docs/Mercurial_FAQ#How_can_I_g
>enerate_a_patch_for_somebody_else_to_check-in_for_me.3F" title="/>enerate_a_patch_for_somebody_else_to_check-in_for_me.3F" title="/
>en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_>en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_
>else_to_check-in_for_me.3F">How can I generate a patch</a>" for m>else_to_check-in_for_me.3F">How can I generate a patch</a>" for m
>ore information about creating patches which are formatted proper>ore information about creating patches which are formatted proper
>ly for easy review.&nbsp; See <a href="/en-US/docs/Developer_Guid>ly for easy review. See <a href="/en-US/docs/Developer_Guide/Revi
>e/Reviewer_Checklist">"Reviewer Checklist"</a> for a list of some>ewer_Checklist">"Reviewer Checklist"</a> for a list of some thing
> things that reviewers will check for.>s that reviewers will check for.
n41      Each patch should represent a single complete change. Separn41      Each patch should represent a single complete change. Separ
>ate distinct changes into multiple patches. If your change result>ate distinct changes into multiple patches. If your change result
>s in a large, complex patch, then see whether it can be broken do>s in a large, complex patch, then see whether it can be broken do
>wn into <a href="https://secure.phabricator.com/book/phabflavor/a>wn into <a href="https://secure.phabricator.com/book/phabflavor/a
>rticle/writing_reviewable_code/#many-small-commits">smaller, easy>rticle/writing_reviewable_code/#many-small-commits">smaller, easy
> to understand patches representing complete steps</a>, applied o> to understand patches representing complete steps</a>, applied o
>n top of each other (the <a href="/en/Mercurial_Queues" title="en>n top of each other (the <a href="/en-US/docs/Mercurial_Queues" t
>/Mercurial Queues">mq extension</a> for mercurial can be of help >itle="/en-US/docs/Mercurial Queues">mq extension</a> for mercuria
>here). This will make it easier to review your changes, <a class=>l can be of help here). This will make it easier to review your c
>"external" href="http://groups.google.com/group/mozilla.dev.plann>hanges, <a class="external" href="http://groups.google.com/group/
>ing/msg/2f99460f57f776ef?hl=en" title="http://groups.google.com/g>mozilla.dev.planning/msg/2f99460f57f776ef?hl=en" title="http://gr
>roup/mozilla.dev.planning/msg/2f99460f57f776ef?hl=en">leading to >oups.google.com/group/mozilla.dev.planning/msg/2f99460f57f776ef?h
>quicker reviews</a> and better confidence in the review.>l=en">leading to quicker reviews</a> and better confidence in the
 > review.
n47      All changes must be tested. In most cases, an <a class="intn47      All changes must be tested. In most cases, an <a class="int
>ernal" href="/en/Mozilla_automated_testing" title="En/Mozilla aut>ernal" href="/en-US/docs/Mozilla_automated_testing" title="/en-US
>omated testing">automated test</a> is required for every change t>/docs/Mozilla automated testing">automated test</a> is required f
>o the code. In some cases where it is not possible to write an au>or every change to the code. In some cases where it is not possib
>tomated test, manual tests called <a class="internal" href="/en/L>le to write an automated test, manual tests called <a class="inte
>itmus_tests" title="En/Litmus tests">Litmus tests</a> are used.>rnal" href="/en-US/docs/Litmus_tests" title="/en-US/docs/Litmus t
 >ests">Litmus tests</a> are used.
n50      Ensure that the change has not caused regressions by runninn50      Ensure that the change has not caused regressions by runnin
>g the automated test suite, locally, or using the <a class="link->g the automated test suite, locally, or using the <a class="link-
>https" href="https://wiki.mozilla.org/Build:TryServer" title="htt>https" href="https://wiki.mozilla.org/Build:TryServer" title="htt
>ps://wiki.mozilla.org/Build:TryServer">Mozilla try server</a>. A >ps://wiki.mozilla.org/Build:TryServer">Mozilla try server</a>. A 
>module owner or developer on IRC&nbsp;may be willing to submit jo>module owner or developer on IRC may be willing to submit jobs fo
>bs for those without try server privileges.>r those without try server privileges.
n68      See <a class="external" href="http://weblog.latte.ca/blake/n68      See <a class="external" href="http://weblog.latte.ca/blake/
>employment/mozilla/thunderbird/reviewerChooser" title="http://web>employment/mozilla/thunderbird/reviewerChooser" title="http://web
>log.latte.ca/blake/employment/mozilla/thunderbird/reviewerChooser>log.latte.ca/blake/employment/mozilla/thunderbird/reviewerChooser
>">"Who should review my change?"</a> for a way to find a reviewer>">"Who should review my change?"</a> for a way to find a reviewer
>. For more information about the review process, see the <a class>. For more information about the review process, see the <a class
>="internal" href="/en/Code_Review_FAQ" title="En/Code Review FAQ">="internal" href="/en-US/docs/Code_Review_FAQ" title="/en-US/docs
>>code review FAQ</a>. If your change affects user interface, see >/Code Review FAQ">code review FAQ</a>. If your change affects use
>"<a href="/En/Developer_Guide/Requesting_feedback_and_ui-review_f>r interface, see "<a href="/en-US/docs/Developer_Guide/Requesting
>or_desktop_Firefox_front-end_changes" title="Requesting feedback >_feedback_and_ui-review_for_desktop_Firefox_front-end_changes" ti
>and ui-review for desktop Firefox front-end changes">Requesting f>tle="Requesting feedback and ui-review for desktop Firefox front-
>eedback and ui-review for desktop Firefox front-end changes</a>".>end changes">Requesting feedback and ui-review for desktop Firefo
 >x front-end changes</a>".
n71      <img align="right" alt="request-review.png" class="internaln71      <img align="right" alt="request-review.png" class="internal
> rwrap" src="/@api/deki/files/3598/=request-review.png" style="wi> rwrap" src="/@api/deki/files/3598/=request-review.png" style="wi
>dth: 519px; height: 149px; border: 1px solid black;">To request a>dth: 519px; height: 149px; border: 1px solid black;">To request a
> review or super-review, click the Details link for the attachmen> review or super-review, click the Details link for the attachmen
>t in bugzilla. On the left side are dropdowns with various labels>t in bugzilla. On the left side are dropdowns with various labels
>.&nbsp; Find the&nbsp;"review" entry and change the flag to "?", >. Find the "review" entry and change the flag to "?", and enter t
>and enter the email address of the module owner or peer who will >he email address of the module owner or peer who will be reviewin
>be reviewing the patch. Remember to submit changes!>g the patch. Remember to submit changes!
n88      It is unusual for patches to be perfect the first time. Then88      It is unusual for patches to be perfect the first time. The
> reviewer may mark "review-" and list problems that must be addre> reviewer may mark "review-" and list problems that must be addre
>ssed before the patch can be accepted. Please remember that reque>ssed before the patch can be accepted. Please remember that reque
>sting revisions is not meant to discourage participation, but rat>sting revisions is not meant to discourage participation, but rat
>her to encourage the best possible resolution of a bug.&nbsp; Car>her to encourage the best possible resolution of a bug. Carefully
>efully work through the changes that the reviewer recommends, att> work through the changes that the reviewer recommends, attach a 
>ach a new patch and request review again.>new patch and request review again.
n91      Sometimes a reviewer will grant conditional review by markin91      Sometimes a reviewer will grant conditional review by marki
>ng "review+" but indicating minor changes that must be made, such>ng "review+" but indicating minor changes that must be made, such
> as spelling or indentation fixes. All recommended corrections sh> as spelling or indentation fixes. All recommended corrections sh
>ould be made, but a re-review is unnecessary. Make the changes, a>ould be made, but a re-review is unnecessary. Make the changes, a
>ttach the revised version, and use the relevant checkbox on the a>ttach the revised version, and use the relevant checkbox on the a
>ttachment page to render the original patch obsolete. If there is>ttachment page to render the original patch obsolete. If there is
> any confusion about the revisions, another review should be requ> any confusion about the revisions, another review should be requ
>ested.&nbsp;>ested.
n100      In many open source projects, developers will accept patchen100      In many open source projects, developers will accept patche
>s in an unfinished state, finish them and apply them.&nbsp; In Mo>s in an unfinished state, finish them and apply them. In Mozilla'
>zilla's culture, <strong>the reviewer will only review and commen>s culture, <strong>the reviewer will only review and comment on a
>t on a patch</strong>.&nbsp; If the submitter declines to make th> patch</strong>. If the submitter declines to make the revisions,
>e revisions, the patch will sit unless and until someone else cho> the patch will sit unless and until someone else chooses to take
>oses to take up the effort.> up the effort.
n113        Please ensure that the patch is <a href="/en/Creating_a_pn113        Please ensure that the patch is <a href="/en-US/docs/Crea
>atch_that_can_be_checked_in" title="https://developer.mozilla.org>ting_a_patch_that_can_be_checked_in" title="/en-US/docs/Creating_
>/en/Creating_a_patch_that_can_be_checked_in">properly formatted</>a_patch_that_can_be_checked_in">properly formatted</a> to make it
>a> to make it as easy as possible for a committer to check in you> as easy as possible for a committer to check in your patch.
>r patch. 
n120      If you're committing yourself, please remember to follow <an120      If you're committing yourself, please remember to follow <a
> href="/En/Developer_Guide/Committing_Rules_and_Responsibilities"> href="/en-US/docs/Developer_Guide/Committing_Rules_and_Responsib
> title="en/Developer Guide/Committing Rules and Responsibilities">ilities" title="/en-US/docs/Developer Guide/Committing Rules and 
>>Committing Rules and Responsibilities</a>.>Responsibilities">Committing Rules and Responsibilities</a>.
n126      It could be that your code causes functional or performancen126      It could be that your code causes functional or performance
> regressions. There is a tight <a class="external" href="http://w> regressions. There is a tight <a class="external" href="http://w
>ww.mozilla.org/hacking/regression-policy.html" title="http://www.>ww.mozilla.org/hacking/regression-policy.html" title="http://www.
>mozilla.org/hacking/regression-policy.html">policy</a> on perform>mozilla.org/hacking/regression-policy.html">policy</a> on perform
>ance regressions in particular, which means your code may well en>ance regressions in particular, which means your code may well en
>d up being backed out and you will have to fix it and resubmit it>d up being backed out and you will have to fix it and resubmit it
>. Regressions mean the tests (which you ran before checking in, d>. Regressions mean the tests (which you ran before checking in, d
>idn't you)&nbsp;are not comprehensive enough, so a resubmitted pa>idn't you) are not comprehensive enough, so a resubmitted patch o
>tch or a patch to fix the regression should be accompanied by app>r a patch to fix the regression should be accompanied by appropri
>ropriate tests.>ate tests.
t131    <p>t
132      {{ languages( { "ja": "ja/Getting_your_patch_in_the_tree" }
> ) }} 
133    </p>

Back to History