Release Management/Uplift rules: Difference between revisions

(fxos edits)
(add a link to "how to request an uplift" page)
 
(22 intermediate revisions by 9 users not shown)
Line 1: Line 1:
{{DISPLAYTITLE:Patch uplifting rules}}


This page describes the rules applied by the Release Team to uplift (aka backport) a patch from a development branch to a more stable branch. For example, taking a patch that fixes a bug in Nightly and applying it to the Beta branch. The [[Release_Management/Tracking_rules|release tracking rules]] page may also be helpful for understanding how release management makes decisions.


This page describes the rules applied by the Release Team to uplift a patch in one of the release.
All [https://firefox-source-docs.mozilla.org/contributing/how_to_submit_a_patch.html regular guidelines] for changes apply.


All [https://developer.mozilla.org/en-US/docs/Developer_Guide/How_to_Submit_a_Patch regular guidelines] for changes apply.
=General=
 
== Requesting Uplift via Phabricator ==
 
See [[Release_Management/Requesting_an_Uplift|requesting an uplift]] for how to request an uplift via Phabricator. This is the preferred workflow for requesting uplifts and replaces the Bugzilla workflow.
 
== Requesting Uplift via Bugzilla==


=General=
* Uplifts are requested in the ''Details'' page of the attachment, ''Flags'' section which should be updated.  
* Uplifts are requested in the ''Details'' page of the attachment, ''Flags'' section which should be updated.  
* If a patch is considered as risky and impact both aurora and beta, please make sure that the other responsible is aware of the uplift.
* For a risky patch, don't hesitate to set the "qe-verify" flag to "+" to make sure someone will check if the bug is actually fixed.
* For a risky patch, don't hesitate to set the keyword "verifyme" to make sure someone will check if the bug is actually fixed.
* Please ensure the patch will graft cleanly to respective repo (Beta/Release/ESR)
** If the patch does not graft cleanly, please attach a rebased patch.
* The name/nickname of the release manager who approved the uplift will be added in the commit message with ''a='' (example: ''a=foobar'')
* The name/nickname of the release manager who approved the uplift will be added in the commit message with ''a='' (example: ''a=foobar'')
* Release Management regularly monitors pending uplift requests. Release Management may reject an uplift request but otherwise the following will apply:
** Beta uplift requests are approved/rejected daily during the Beta cycle. Uplifted patches are released in the next Beta build, the Release Manager will add comments when approving and mentions the next Beta build.
** Release uplift requests approval will depend on where we are in the cycle. If the request is not a RC respin driver or a dot release driver, the request will pend as a potential ride-along until the Release Manager identifies an appropriate RC respin/dot release to include the uplift.


 
= Guidelines on approval comments for Beta and Release =
= Guidelines on approval comments =
Requesting an uplift request is performed by setting the relevant flag, i.e. approval-mozilla-beta, approval-mozilla-release, or approval-mozilla-esr,  on an attachment to "?"
* [Feature/regressing bug #]: If you this is a fallout from a feature/bug, or a backout please state the reason along with the bug number
A form is automatically created to add information in a comment for the uplift request.
* [User impact if declined]: In addition to the STR (steps to reproduce) reported in the bug if you can explain on a deeper level aspect of how an end user would be impacted with/without your change
* [User impact if declined]: In addition to the STR (steps to reproduce) reported in the bug if you can explain on a deeper level aspect of how an end user would be impacted with/without your change
* [Describe test coverage new/current, TBPL]: What kind of testing was completed here. Manual and/or automated ? Any details about what scenarios the tests cover? If the patch landed on a master/central branch or any other branch did we get verification from reporter or QA.
* [Is this code covered by automated tests?] Yes/No/Unknown
* [Risks and why]: No risk/Zero risk are unacceptable here. Even a single line of code change could be damaging!, yes we've seen that
* [Has the fix been verified in Nightly?] Was the fix verified in a nightly build, e.g. by the reporter or QA?
** When saying something is "low","medium", "risky" please justify
* [Needs manual test from QE?] Yes/No
** Eg : Low risk because its a one line CSS change impacting only settings page
** [If yes, steps to reproduce] Either point to an existing comment listing STR or elaborate here directly.
** Eg : Medium, given the code complexity and integration with other areas of code that might be impacted. Expect regressions in areas like..
* [List of other uplifts needed] If this patch depends on other changes that are not present on the target branch, list them here (and request approval in the other bugs as well)
** Eg : risky, given the complex nature the bake time we have have on master/central or other branches. High rate of fallouts/regression. Could be mitigated by more manual testing in areas or running some targeted test cases..
* [Risk to taking this patch]: Low/Medium/High
* [String/UUID change made/needed]: Please answer this as "none" if no string changes were made
* [Why is the change risky/not risky? (and alternatives if risky)]
** When saying something is "low", "medium", or "high" risk please justify
** E.g.: Low risk because its a one line CSS change impacting only settings page
** E.g.: Medium, given the code complexity and integration with other areas of code that might be impacted. Expect regressions in areas like...
** E.g.: Risky, given the complex nature the bake time we have have on central or other branches. High rate of fallouts/regression. Could be mitigated by more manual testing in areas or running some targeted test cases...  If the issue could be worked around by disabling a certain feature, or with a different, less-risky, patch for uplift, mention this here.
* [String changes made/needed] Please answer this as "none" if no string changes were made.  String changes need to be approved by a l10n driver since they impact the work of localization teams.
* [Is Android affected?] Does this fix also impact Android applications? [https://wiki.mozilla.org/Mobile/GeckoView GeckoView] Meaning as well as a desktop release, Release Management should create an [https://mozac.org/ android-components] build and follow up with Fenix/Focus.


= Firefox (Desktop and Mobile) =
= Firefox (Desktop and Mobile) =
==Aurora Uplift (approval-mozilla-aurora)==
==Beta Uplift (approval-mozilla-beta)==
* Can accept IDL (with UUID bump)  changes as we do not lock down binary for addons until Beta
* No string changes unless late-l10n awareness and OK given
* Must be landed on mozilla-central, or reason given for direct-to-branch uplift
* Must be landed on mozilla-central, or reason given for direct-to-branch uplift
* No new feature landing
* No disruptive important refactoring
* No massive code changes
==Beta Uplift (approval-mozilla-beta)==
* Must be landed on mozilla-central as well as mozilla-aurora, or reason given for direct-to-branch uplift
* Ideally reproducible by QA so easily verified
* Ideally reproducible by QA so easily verified
* Has 'baked' on m-c/aurora and demonstrated decrease in crash or reproducibility
* Has 'baked' on m-c and demonstrated decrease in crash or reproducibility
* No string changes
* No string changes, unless approved by l10n driver
* No changes to UUID in .idl files (will break addon compatibility)


Changes can be:
Changes can be:
Line 43: Line 51:
* Top-crashers
* Top-crashers
* Recent regressions
* Recent regressions
* Low risk improvements early in the cycle
* Enabling features that have been validated/tested by QA, or have already shipped e.g. through a normandy rollout.


The closer to the release the more careful uplift should be done.
The closer to the release the more careful an uplift should be done.


==Release Uplift==
==Release Uplift (approval-mozilla-release) ==
* Must be a part of a known-issue respin or NPTOB (not part of the build) config changes needed to support build infra
Some issues are bad enough that we don't want to wait until the next major release. We do major releases every 4 weeks. Most fixes can wait that long.  Candidates for uplift to release may be drivers of a dot release, or may be "ridealongs" nice but not crucial to have.  If it would be a release blocker, it might be a good dot release driver.


=Firefox OS gaia/gecko uplifts=
Each dot release will mean annoyance for users, and will also result in an increased chance for some users of being "orphaned" on that version.


==approval-gaia-v2.x=/approval-gecko-b2g37==
"Ridealongs" or any extra fixes increase the chance that we will cause a new regression that may need another fix and uplift, or even drive another dot release.
* For any gaia/gecko change's landing on the 2.x release, please request approval to land
* Please note, the closer we try to wrap up stabilization mode the more stricter we'd be on taking any changes


=== Criteria to request approval (Guidelines) ===
For uplift requests to release, please give as much data as possible to help with the decision. It is also helpful if you can specify if your fix affects desktop, android, or both.
* Patch must be landed and Resolved on master/central
* any bug that has a high user impact
* String changes may not be approved after the string freeze dealine
* a low risk Polish change : Visual polish - colors, sizes, icons, alignment, etc
* functional-polish: Example, Notification tray displaying that your microphone is active, but onclick does not do do anything. Ideally it should take to to microphone settings if I wanted to play with it..It's not really a bug, but it could be better. And it requires code changes, not just visual work.
* papercuts: It's for issues found during dogfooding/testing that are visually or functionally annoying or otherwise sub-par, but not enough to block the release
* Performance/crash-fixes : Depending on the risk evaluation and the reward we get ny uplifting these will be highly considered depending on where we are in the release cycle
* If you a requesting approval on a gaia/gecko patch that needs to land on 2.x that
** Set approval-gaia-v2.x: ? and making sure to fill the populated set of questions [Approval Request Comment] that come up on the attachment
[[File:Gaia-approval.png|800px|center]]


Examples include:
* Major top crash (above or near the level for oom crashes). Provide evidence of impact.
* High volume startup crash.
* Security issue that doesn't need a chemspill, but that is bad enough to drive a dot release
* Functional regression with a broad impact (We can see from telemetry it's affecting many users, or we have many reports from support.mozilla.org, input.m.o, or other user reports)
* Problem in a major feature


* To uplift to release without relman approval, your change must be a part of a known-issue respin or NPOTB (not part of the build) config changes needed to support build infra
==ESR Uplift (approval-mozilla-esrXX) ==
We do an ESR minor releases every 4 weeks. These releases do not include new features and are reserved for stability/security fixes only. ESR uplift requests follow a similar uplift request process as Beta/Release, but with a variation on the information provided with the uplift.


=Special cases=
=Special cases=


* In the context of ''add-on SDK'' modifications, when an uplift is requested, it is normal not to find the land in m-c (because the m-c changeset is not posted in the bug report)
* If patches only make changes to tests, test harnesses or anything else that does not affect the shipped builds, they may land with self approval (use a=testonly, a=npotb etc).
* If patches only make changes to tests, test harnesses or anything else that does not affect the shipped builds, they may land with self approval (use a=testonly, a=npotb etc).
** In that case, instead of setting the "approval-mozilla-beta" flag on the patch, you can add "[checkin-needed-beta]" in the bug's "whiteboard" field.


=Security fixes=
=Security fixes=
As described in [[Security/Bug_Approval_Process]]
As described in [[Security/Bug_Approval_Process]]
If the bug whiteboard contains a deadline, the uplift should be granted only after this date.
If the bug whiteboard contains a deadline, the uplift should be granted only after this date.
=Uplift review criteria/best practices=
* Size of change, patch
* Which component is this fix in?
** Low risk: CSS, front-end, build system
** Medium risk: Telemetry (adding probes on hot path is perf risk) or changing the data collection
** High risk: Graphics, layout, printing, DOM, JS, Performance related changes (56% caused regressions), Mem allocator, GCs, Search defaults
* Lots of discussion in comments - be more cautious
* Lots of patch review attempts -  be more cautious
* Test status
** Verification on previous channels helps!
* QA verification
* Timing of uplift during beta cycle
** Uplifts during late cycle need more caution
** Uplifts early in the cycle can afford the luxury of less scrutiny
** For RC week and the week before, consider only dot release worthy uplifts
** Be cautious with trivial ride-alongs
* Automated tests
** More automated tests the better
* Code coverage
** Consider if there's adequate automated test coverage to understand the impact of the change or does the change require bake-time on nightly
* Crash, memleaks, hangs, perf fixes
** High volume crash fixes and fixes that are verified from other channels helps
** Due to high impact, these need more scrutiny
* Data collection changes
** Mandate data-review (might use Phabricator commit hook / subscription for that)
* Localization (l10n) changes
** Loop in l10n team for their review
** Changes would include new strings


[[category:Release_Management|U]]
[[category:Release_Management|U]]
[[Category:Release_Management:Processes|Uplift]]

Latest revision as of 20:48, 22 March 2023


This page describes the rules applied by the Release Team to uplift (aka backport) a patch from a development branch to a more stable branch. For example, taking a patch that fixes a bug in Nightly and applying it to the Beta branch. The release tracking rules page may also be helpful for understanding how release management makes decisions.

All regular guidelines for changes apply.

General

Requesting Uplift via Phabricator

See requesting an uplift for how to request an uplift via Phabricator. This is the preferred workflow for requesting uplifts and replaces the Bugzilla workflow.

Requesting Uplift via Bugzilla

  • Uplifts are requested in the Details page of the attachment, Flags section which should be updated.
  • For a risky patch, don't hesitate to set the "qe-verify" flag to "+" to make sure someone will check if the bug is actually fixed.
  • Please ensure the patch will graft cleanly to respective repo (Beta/Release/ESR)
    • If the patch does not graft cleanly, please attach a rebased patch.
  • The name/nickname of the release manager who approved the uplift will be added in the commit message with a= (example: a=foobar)
  • Release Management regularly monitors pending uplift requests. Release Management may reject an uplift request but otherwise the following will apply:
    • Beta uplift requests are approved/rejected daily during the Beta cycle. Uplifted patches are released in the next Beta build, the Release Manager will add comments when approving and mentions the next Beta build.
    • Release uplift requests approval will depend on where we are in the cycle. If the request is not a RC respin driver or a dot release driver, the request will pend as a potential ride-along until the Release Manager identifies an appropriate RC respin/dot release to include the uplift.

Guidelines on approval comments for Beta and Release

Requesting an uplift request is performed by setting the relevant flag, i.e. approval-mozilla-beta, approval-mozilla-release, or approval-mozilla-esr, on an attachment to "?" A form is automatically created to add information in a comment for the uplift request.

  • [User impact if declined]: In addition to the STR (steps to reproduce) reported in the bug if you can explain on a deeper level aspect of how an end user would be impacted with/without your change
  • [Is this code covered by automated tests?] Yes/No/Unknown
  • [Has the fix been verified in Nightly?] Was the fix verified in a nightly build, e.g. by the reporter or QA?
  • [Needs manual test from QE?] Yes/No
    • [If yes, steps to reproduce] Either point to an existing comment listing STR or elaborate here directly.
  • [List of other uplifts needed] If this patch depends on other changes that are not present on the target branch, list them here (and request approval in the other bugs as well)
  • [Risk to taking this patch]: Low/Medium/High
  • [Why is the change risky/not risky? (and alternatives if risky)]
    • When saying something is "low", "medium", or "high" risk please justify
    • E.g.: Low risk because its a one line CSS change impacting only settings page
    • E.g.: Medium, given the code complexity and integration with other areas of code that might be impacted. Expect regressions in areas like...
    • E.g.: Risky, given the complex nature the bake time we have have on central or other branches. High rate of fallouts/regression. Could be mitigated by more manual testing in areas or running some targeted test cases... If the issue could be worked around by disabling a certain feature, or with a different, less-risky, patch for uplift, mention this here.
  • [String changes made/needed] Please answer this as "none" if no string changes were made. String changes need to be approved by a l10n driver since they impact the work of localization teams.
  • [Is Android affected?] Does this fix also impact Android applications? GeckoView Meaning as well as a desktop release, Release Management should create an android-components build and follow up with Fenix/Focus.

Firefox (Desktop and Mobile)

Beta Uplift (approval-mozilla-beta)

  • Must be landed on mozilla-central, or reason given for direct-to-branch uplift
  • Ideally reproducible by QA so easily verified
  • Has 'baked' on m-c and demonstrated decrease in crash or reproducibility
  • No string changes, unless approved by l10n driver

Changes can be:

  • Performance improvement (proven, need real numbers)
  • Top-crashers
  • Recent regressions
  • Low risk improvements early in the cycle
  • Enabling features that have been validated/tested by QA, or have already shipped e.g. through a normandy rollout.

The closer to the release the more careful an uplift should be done.

Release Uplift (approval-mozilla-release)

Some issues are bad enough that we don't want to wait until the next major release. We do major releases every 4 weeks. Most fixes can wait that long. Candidates for uplift to release may be drivers of a dot release, or may be "ridealongs" nice but not crucial to have. If it would be a release blocker, it might be a good dot release driver.

Each dot release will mean annoyance for users, and will also result in an increased chance for some users of being "orphaned" on that version.

"Ridealongs" or any extra fixes increase the chance that we will cause a new regression that may need another fix and uplift, or even drive another dot release.

For uplift requests to release, please give as much data as possible to help with the decision. It is also helpful if you can specify if your fix affects desktop, android, or both.

Examples include:

  • Major top crash (above or near the level for oom crashes). Provide evidence of impact.
  • High volume startup crash.
  • Security issue that doesn't need a chemspill, but that is bad enough to drive a dot release
  • Functional regression with a broad impact (We can see from telemetry it's affecting many users, or we have many reports from support.mozilla.org, input.m.o, or other user reports)
  • Problem in a major feature
  • To uplift to release without relman approval, your change must be a part of a known-issue respin or NPOTB (not part of the build) config changes needed to support build infra

ESR Uplift (approval-mozilla-esrXX)

We do an ESR minor releases every 4 weeks. These releases do not include new features and are reserved for stability/security fixes only. ESR uplift requests follow a similar uplift request process as Beta/Release, but with a variation on the information provided with the uplift.

Special cases

  • If patches only make changes to tests, test harnesses or anything else that does not affect the shipped builds, they may land with self approval (use a=testonly, a=npotb etc).
    • In that case, instead of setting the "approval-mozilla-beta" flag on the patch, you can add "[checkin-needed-beta]" in the bug's "whiteboard" field.

Security fixes

As described in Security/Bug_Approval_Process If the bug whiteboard contains a deadline, the uplift should be granted only after this date.

Uplift review criteria/best practices

  • Size of change, patch
  • Which component is this fix in?
    • Low risk: CSS, front-end, build system
    • Medium risk: Telemetry (adding probes on hot path is perf risk) or changing the data collection
    • High risk: Graphics, layout, printing, DOM, JS, Performance related changes (56% caused regressions), Mem allocator, GCs, Search defaults
  • Lots of discussion in comments - be more cautious
  • Lots of patch review attempts - be more cautious
  • Test status
    • Verification on previous channels helps!
  • QA verification
  • Timing of uplift during beta cycle
    • Uplifts during late cycle need more caution
    • Uplifts early in the cycle can afford the luxury of less scrutiny
    • For RC week and the week before, consider only dot release worthy uplifts
    • Be cautious with trivial ride-alongs
  • Automated tests
    • More automated tests the better
  • Code coverage
    • Consider if there's adequate automated test coverage to understand the impact of the change or does the change require bake-time on nightly
  • Crash, memleaks, hangs, perf fixes
    • High volume crash fixes and fixes that are verified from other channels helps
    • Due to high impact, these need more scrutiny
  • Data collection changes
    • Mandate data-review (might use Phabricator commit hook / subscription for that)
  • Localization (l10n) changes
    • Loop in l10n team for their review
    • Changes would include new strings