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.
All regular guidelines for changes apply.
- Uplifts are requested in the Details page of the attachment, Flags section which should be updated.
- If a patch is considered as risky and impacts beta, please make sure that the other responsible is aware of the uplift.
- For a risky patch, don't hesitate to set the keyword "verifyme" to make sure someone will check if the bug is actually fixed.
- The name/nickname of the release manager who approved the uplift will be added in the commit message with a= (example: a=foobar)
Guidelines on approval comments
- [Feature/regressing bug #]: If this is a fallout from a feature/bug, or a backout please state the reason along with the bug number
- [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?
- [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
- When saying something is "low", "medium", or "risky" please justify
- Eg : Low risk because its a one line CSS change impacting only settings page
- Eg : Medium, given the code complexity and integration with other areas of code that might be impacted. Expect regressions in areas like..
- 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..
- [String/UUID change made/needed]: Please answer this as "none" if no string changes were made
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
Changes can be:
- Performance improvement (proven, need real numbers)
- Recent regressions
The closer to the release the more careful uplift should be done.
Some issues are bad enough that we don't want to wait till the next major release. We do major releases every 6-8 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, fennec, or both.
- 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
- 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).
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
- 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
- In Quantum, we used second review from eng mngr during RC week