blob: 3bf77655a88a3d59b068782a78d018675791d863 [file] [log] [blame] [view]
Abdul Syede21aa1b2017-07-06 17:13:061# Merge Request Process
2
3[TOC]
4
5## tl;dr
6
Alex Mineerc5b63022021-08-26 21:36:057* Release managers (and delegates like the security team) must review all
8 merges made to release branches
9* Merge criteria become more strict as the stable release date approaches; use
10 Chromium Dash's [Branches page](https://chromiumdash.appspot.com/branches) to
11 understand which branches are active and what merges are acceptable for each
12 branch
13* Ensure your change is [safe to merge](#verifying-eligibility-and-safety)
14 before initiating the merge review process unless it's time-sensitive
Amy Ressler373f77722024-02-03 00:39:4715* Use Chromium Issue Tracker's [project queries](#monitoring-merge-requests) to
16 track your approved merges as well as your pending requests
Alex Mineerc5b63022021-08-26 21:36:0517* Use Gerrit or git to land your merge only after it's been approved
Abdul Syede21aa1b2017-07-06 17:13:0618
19## Introduction
20
Alex Mineerc5b63022021-08-26 21:36:0521Chromium is a main-first development team; generally, all code should land on
22main then roll out to stable users only after the milestone containing the code
23is branched, stabilized and shipped to the stable channel (to learn more about
24the release cycle, click
25[here](https://chromium.googlesource.com/chromium/src.git/+/main/docs/process/release_cycle.md)).
26This is because merging (also known as cherry-picking) code to an older release
27branch introduces risk and costs time across the team. However, there are times
28when the benefits outweigh the costs and a merge might be appropriate, e.g. to
29fix a web platform regression, address a crash or patch a security vulnerability.
Abdul Syede21aa1b2017-07-06 17:13:0630
Alex Mineerc5b63022021-08-26 21:36:0531To ensure we make the right decisions, release managers leverage a merge review
32process to evaluate each request. They'll ask questions about the reason you
33would like to merge a change and the risk of the merge itself, and you'll work
34together to make a judgement call on whether or not the merge should be
35approved or rejected.
Abdul Syede21aa1b2017-07-06 17:13:0636
Alex Mineerc5b63022021-08-26 21:36:0537Generally, merges follow these high-level steps:
Abdul Syede21aa1b2017-07-06 17:13:0638
Amy Ressler373f77722024-02-03 00:39:4739* Developers update bug with relevant details and request a merge by updating
40 the *Merge* field with *Request-##* label, then wait for review.
Alex Mineerc5b63022021-08-26 21:36:0541* Release managers and automation review and approve, reject, or ask
Amy Ressler373f77722024-02-03 00:39:4742 questions about the merge within two business days.
43* Developers wait for review and, if approved, land the merge ASAP.
Abdul Syede21aa1b2017-07-06 17:13:0644
Alex Mineerc5b63022021-08-26 21:36:0545For details on each step, see below.
Abdul Syede21aa1b2017-07-06 17:13:0646
Amy Ressler373f77722024-02-03 00:39:4747**NOTE:** Because security issues (identified with *Type=Vulnerability*) follow
48a more complex flow, you may simply mark security issues as *Fixed* in the
49issue tracker and [automation](#security-merge-triage) will handle the
50remainder of the merge request process flow for you; simply process the merge
51if it is requested and approved.
Abdul Syede21aa1b2017-07-06 17:13:0652
Alex Mineerc5b63022021-08-26 21:36:0553## Requesting a merge
Abdul Syede21aa1b2017-07-06 17:13:0654
Alex Mineerc5b63022021-08-26 21:36:0555### Verifying eligibility and safety
Abdul Syede21aa1b2017-07-06 17:13:0656
Alex Mineerc5b63022021-08-26 21:36:0557Before requesting a merge, first ensure your change is a good merge candidate:
Abdul Syede21aa1b2017-07-06 17:13:0658
Alex Mineerc5b63022021-08-26 21:36:0559* Ensure it meets the merge criteria (via
60 [Chromium Dash](https://chromiumdash.appspot.com/branches)) of the branch(es)
61 you'd like to merge to; merge criteria become more strict the older the
62 branch is, more details on criteria [below](#merge-criteria-phases)
63* Verify merging the change to an older branch would be safe, e.g. unlikely to
64 introduce new regressions, no major merge conflicts, automated test coverage
65 present, etc; chat with your TL for input if you're not sure
66* Confirm your change fixes the issue at hand, preferably by testing on and
67 monitoring the canary channel for 24 hours post-release (see
68 [Chromium Dash](https://chromiumdash.appspot.com/commits) to determine if
69 your change has shipped)
Abdul Syede21aa1b2017-07-06 17:13:0670
Alex Mineerc5b63022021-08-26 21:36:0571 * You may skip this step if a release manager or security team member has
72 told you that the merge is urgent, e.g. is actively blocking a release
Alex Mineer40d33c32018-03-15 23:45:1473
Amy Ressler373f77722024-02-03 00:39:4774### Updating Chromium Issue Tracker
Abdul Syede21aa1b2017-07-06 17:13:0675
Alex Mineerc5b63022021-08-26 21:36:0576Next, ensure you have a crbug/ (generally the bug being fixed by the merge)
77with the following information present and accurate:
Abdul Syede21aa1b2017-07-06 17:13:0678
Alex Mineerc5b63022021-08-26 21:36:0579* Title and description clearly describing the bug being fixed
Amy Ressler373f77722024-02-03 00:39:4780* Priority (*P#*), OS (*OS-OS*) and target milestone(s) (*Target-##*)
Alex Mineerc5b63022021-08-26 21:36:0581* Owner, generally the person requesting / performing the merge
82* [Release block label](./release_blockers.md) if applicable
83 (*ReleaseBlock=Channel*)
84* Issue status:
Abdul Syede21aa1b2017-07-06 17:13:0685
Alex Mineerc5b63022021-08-26 21:36:0586 * Fixed: You're confident the issue is fixed on main, e.g. you've
87 locally built and tested the issue, no additional crash reports are
88 generated after the fix was released, etc (most issues)
89 * Assigned / Started: Diagnostic merges only, e.g. to merge code to track
90 down the root cause of an issue that only exists on branch
Abdul Syede21aa1b2017-07-06 17:13:0691
Alex Mineerc5b63022021-08-26 21:36:0592### Applying merge request label
Abdul Syede21aa1b2017-07-06 17:13:0693
Amy Ressler373f77722024-02-03 00:39:4794Once you've verified all the above, you're ready to request a merge! Simply
95update the *Merge* field with *Request-##* on the issue (where ## indicates the
96milestone you'd like to merge to), and use multiple tags for multiple
97milestones, e.g.*Request-121*, *Request-122* for M121 and M122. Please also copy
98the following questions and answer them in a comment on the issue:
Alan Cuttercba85fbf2019-10-18 06:21:4799
Alex Mineerc5b63022021-08-26 21:36:051001. Why does your merge fit within the merge criteria for these milestones
101 ([Chrome Browser](https://chromiumdash.appspot.com/branches),
102 [Chrome OS](https://goto.google.com/cros-release-branch-merge-guidelines))?
1032. What changes specifically would you like to merge? Please link to Gerrit.
1043. Have the changes been released and tested on canary?
1054. Is this a new feature? If yes, is it behind a Finch flag and are
106 experiments active in any release channels?
1075. [Chrome OS only]: Was the change reviewed and approved by the
108 [Eng Prod Representative](https://goto.google.com/cros-engprodcomponents)?
David Benjamin58230aec2022-02-14 22:35:231096. If this merge addresses a major issue in the stable channel, does it require
110 manual verification by the test team? If so, please describe required
111 testing.
Abdul Syede21aa1b2017-07-06 17:13:06112
Alex Mineerc5b63022021-08-26 21:36:05113## Monitoring merge requests
Abdul Syede21aa1b2017-07-06 17:13:06114
Amy Ressler373f77722024-02-03 00:39:47115After you've updated the *Merge* field with *Request-##*, automation will
116evaluate your request and may either approve it, reject it, or pass it along
117to a release manager for manual evaluation; see [here](#merge-request-triage)
118to learn more about this automation. If manual review is required, release
Alex Mineerc5b63022021-08-26 21:36:05119managers strive to answer all merge requests within two business days, but
120extenuating circumstances may cause delays.
Abdul Syede21aa1b2017-07-06 17:13:06121
Alex Mineerc5b63022021-08-26 21:36:05122At this point, following along via bug comments sent by email will always keep
123you in the loop, but you can also use the following saved project queries in
Amy Ressler373f77722024-02-03 00:39:47124Chromium Issue Tracker (The following queries reflect the legacy issue tracker,
125Monorail - bugs.chromium.org, and will be updated once the migration to the new
126[Chromium Issue Tracker](https://issues.chromium.org) is complete.):
Abdul Syede21aa1b2017-07-06 17:13:06127
Alex Mineerc5b63022021-08-26 21:36:05128* [Approved and TBD merges](https://bugs.chromium.org/p/chromium/issues/list?q=owner%3Ame&can=41025836):
129 Merges that require your follow-up, either by landing the relevant
130 merge (if approved) or determining whether or not a merge is actually
131 required and if so, requesting it (if TBD)
132* [Requested merges](https://bugs.chromium.org/p/chromium/issues/list?q=owner%3Ame&can=41025837):
133 Merges that are waiting for input from release managers or automation; feel
134 free to ping bugs that sit in this queue for two business days (assuming you
135 verified that the change was already deployed to canary ahead of requesting a
136 merge)
137* [Rejected and NA merges](https://bugs.chromium.org/p/chromium/issues/list?q=owner%3Ame&can=41025838):
138 Merges that were either rejected by release managers, or not applicable to be
139 merged; generally, no action is needed for these items unless you disagree
140 with a merge's rejection and wish to escalate
141* [All merges](https://bugs.chromium.org/p/chromium/issues/list?q=owner%3Ame&can=41025839):
142 Includes every possible merge state, useful when wanting to find an item you
143 considered for merging but can't recall the state it was last in.
Abdul Syede21aa1b2017-07-06 17:13:06144
Alex Mineerc5b63022021-08-26 21:36:05145For a description of each label used to track the merge process, see the
146appendix [below](#merge-states-and-labels).
Abdul Syede21aa1b2017-07-06 17:13:06147
Alex Mineerc5b63022021-08-26 21:36:05148## Landing an approved merge
Abdul Syede21aa1b2017-07-06 17:13:06149
Alex Mineerc5b63022021-08-26 21:36:05150Once your merge has been approved for a given milestone (via the release
Amy Ressler373f77722024-02-03 00:39:47151manager or automation updating the *Merge* field with *Approved-##*), you have
152two options to land the merge:
Abdul Syede21aa1b2017-07-06 17:13:06153
Alex Mineerc5b63022021-08-26 21:36:05154* Gerrit UI, easiest for clean cherry-picks or those requiring only minor
155 changes
156* git, for more complex cherry-picks and / or when local verification may be
157 beneficial
Alan Cuttere338ed92021-05-06 04:57:19158
Alex Mineerc5b63022021-08-26 21:36:05159Regardless of which method you choose, please ensure you land your cherry-pick
160ASAP so that it can be included in the next release built from the branch; if
161you don't merge your cherry-pick soon after approval, it will eventually be
162rejected for merge.
Abdul Syede21aa1b2017-07-06 17:13:06163
Amy Ressler373f77722024-02-03 00:39:47164Once the cherry-pick has landed a bot will update the *Merge* field with
165*merged-##* if the commit references the issue. If for some reason the commit
166did not reference the issue, update the *Merge* field to *Merged-##* on the
167issue. After the merge is completed the *Approved-##* should be manually
168removed from the *Merge* field.
Ben Wagner96babd4c2022-08-26 16:53:09169
Alex Mineerc5b63022021-08-26 21:36:05170### Using Gerrit UI
Abdul Syede21aa1b2017-07-06 17:13:06171
David Benjamin58230aec2022-02-14 22:35:23172Select the "..." button in the Gerrit UI, then choose "Cherry Pick". When
Alex Mineerc5b63022021-08-26 21:36:05173prompted for a branch, enter *refs/branch-heads/####*, where #### corresponds
174to the release branch you are merging to (available on
mlcuieaa3e3952022-02-14 22:52:06175[Chromium Dash](https://chromiumdash.appspot.com/branches) in the "Chromium"
176column).
Abdul Syede21aa1b2017-07-06 17:13:06177
K. Moon216a9a1192022-08-26 06:02:36178Once the cherry-pick CL is prepared, you can bypass code review (but not
Xinyu Yang8bc1ba72022-11-04 05:01:11179OWNERS approval) within 14 days of the original change by adding the Rubber
K. Moon216a9a1192022-08-26 06:02:36180Stamper bot ([email protected]) as a reviewer. If the
181CL meets the
182[Rubber Stamper criteria](https://chromium.googlesource.com/infra/infra/+/refs/heads/main/go/src/infra/appengine/rubber-stamper/README.md),
183the bot will vote *Bot-Commit+1* to bypass code review. If the CL is marked
184*Auto-Submit+1*, the bot will also submit the CL to the CQ on your behalf.
Ian Barkley-Yeung3af975e2021-04-06 19:00:35185
Alex Mineerc5b63022021-08-26 21:36:05186### Using git
187
188The commands below should set up your environment to be able to successfully
189upload a cherry-pick to a release branch, where *####* corresponds to the
190release branch you are merging to (available on
mlcuieaa3e3952022-02-14 22:52:06191[Chromium Dash](https://chromiumdash.appspot.com/branches) in the "Chromium"
192column):
Alex Mineerc5b63022021-08-26 21:36:05193
194```
195$ gclient sync --with_branch_heads
196$ git fetch
197$ git checkout -b BRANCH_NAME refs/remotes/branch-heads/####
198$ git cl upstream branch-heads/####
199$ git cherry-pick -x COMMIT_HASH_MAIN
Yoav Weiss42be0432022-11-26 19:04:07200$ gclient sync
Alex Mineerc5b63022021-08-26 21:36:05201```
202
203From here, your environment should be ready to adjust the change as required;
204use ninja to build and test your changes, and when ready upload for review:
205
206```
207$ git cl upload
208```
209
210**Adjust the change description** to omit the "Change-Id: ..." line from
211original patch, otherwise you may experience issues when uploading the change
212to Gerrit. Once complete, use Gerrit to initiate review and approval of the
213merge as TBR has been discontinued.
214
215Other tips & tricks when merging with git via release branches:
216* Consider using multiple working directories when creating the release branch
217* Editing the change description to denote this is a merge (e.g. "Merge to
218 release branch" at the top) will help reviewers distinguish between the
219 cherry-pick and the original change
220
221## Merge automation
222
Amy Ressler373f77722024-02-03 00:39:47223The Chrome team has built automation via
224[Blintz](https://www.chromium.org/issue-tracking/autotriage), formerly known as
225Sheriffbot, to assist in several merge flows: security merge triage, general
226merge request triage, and preventing missed merges.
Alex Mineerc5b63022021-08-26 21:36:05227
228### Security merge triage
229
230Given the additional complexity inherent in security merges, the security team
231has built custom automation to handle this flow end to end; simply mark any
232security issue as *Fixed* and Sheriffbot will evaluate applicable milestones,
233determine if merges are required and automatically request them if need be.
234
235### Merge request triage
236
237To reduce release manager toil, Sheriffbot performs the first pass review of
238all merge requests; it may auto-approve the issue if it can detect the issue
239meets the right criteria for the current merge phase (e.g. a ReleaseBlock-Dev
240issue requesting a merge before beta promotion), and it may auto-reject the
Amy Ressler373f77722024-02-03 00:39:47241issue similarly (e.g. a P3 issue requesting a merge post-stable). If it
Alex Mineerc5b63022021-08-26 21:36:05242cannot decide, it will pass the issue to a release manager for manual review.
243
244Generally, Sheriffbot takes action on merge requests only after one of the two
245conditions below are met:
246
247* One or more changelists (via Gitwatcher) are present on the merge request
248 issue, and all changes have been landed for >= 24 hours
249* No changelists are present on the merge request issue, and the merge request
250 label has been applied for >= 24 hours
251
252These conditions help ensure any relevant changelists have had sufficient
253runtime in our canary channel and thus are low risk for introducing a new
254regression onto our release branch.
255
256### Preventing missed merges
257
258To avoid the situation where a critical issue is present on a release branch
259but the fix isn't merged, Sheriffbot evaluates all release-blocking issues
Amy Ressler373f77722024-02-03 00:39:47260targeting a milestone that has already branched and updates the *Merge* field
261with *TBD-##* if the issue was marked as fixed after branch day but hasn't been
262merged. When this occurs, developers should evaluate the issue and either
263request a merge if required (e.g. the fix did miss the release branch point) by
264updating the *Merge* field with *Request-##*, or update the *Merge* field with
265*NA-##* (e.g. the fix is present in the release branch already or the merge is
266unnecessary for other reasons).
Alex Mineerc5b63022021-08-26 21:36:05267
268## Appendix
269
270### Merge criteria phases
271
272The table below describes the different phases that each milestone progresses
273through during its release cycle; this data is available via the
274Chromium Dash [front-end](https://chromiumdash.appspot.com/branches) and
275[API](https://chromiumdash.appspot.com/fetch_milestones).
276
277| Branch Phase | Period Begins | Period Ends | Acceptable Merges Include Fixes For: |
278|--------------------------|-----------------|-----------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
279| branch | M(X) Branch | M(X) Beta | Polish issues for Finch-gated features (no workflow changes), any new regressions, any release blockers, any security issues, any string issues (.GRD changes) |
280| beta | M(X) Beta | M(X) Stable Cut | Non-functional issues for Finch-gated features (e.g. add metrics, fix crash), noticeable new regressions, any release blockers, any security issues, urgent string issues (.GRD changes) |
281| stable_cut | M(X) Stable Cut | M(X) Stable | Urgent new regressions, all release blockers, important security issues (medium severity or higher), emergency string issues (.GRD changes) |
Alex Mineerb201d4a2021-11-04 22:30:29282| stable | M(X) Stable | M(X+1) Stable | Urgent new regressions (especially user reports), urgent release blockers, important security issues (medium severity or higher) requested by the security team |
283| extended (if applicable) | M(X+1) Stable | M(X+2) Stable | Important security issues (medium severity or higher) applicable to any platform supported by Chrome Browser requested by the security team |
Alex Mineerc5b63022021-08-26 21:36:05284
285### Merge states and labels
286
287The table below describes the different merge states and labels used to track
Amy Ressler373f77722024-02-03 00:39:47288them. All *Merge* field labels follow the form *[State]-##*, where ##
289corresponds to the applicable milestone. If multiple merges are required, these
290labels may appear multiple times on the same bug in different states (e.g. the
291*Merge* field could contain both *Approved-122* and *Rejected-121* at the same
292time.
Alex Mineerc5b63022021-08-26 21:36:05293
294| Label / State | Step Owner | Next Steps |
295|---------------|------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
296| Request | Release manager | Automation will review and either approve / reject directly, or pass the review to a release manager for manual evaluation |
297| Review | Release manager | Release manager will evaluate and either approve, reject, or request additional information within two business days |
298| Approved | Issue owner | Issue owner should cherry-pick the fix to the appropriate release branch ASAP |
299| Merged | None | N/A; merge has already been landed, no further work required for given milestone |
Amy Ressler373f77722024-02-03 00:39:47300| Rejected | Issue owner | Issue owner should re-update *Merge* field with *Request-##* to escalate if they feel the merge was erroneously rejected and should be re-evaluated |
301| TBD | Issue owner | Issue owner should evaluate if a merge is required, then remove *TBD-##* from the *Merge* and replace it with *NA-##* (if no merge needed) or *Request-##* (if merge needed) |
mlcuieaa3e3952022-02-14 22:52:06302| NA | None | N/A; merge is not required to the relevant milestone, no further work required for given milestone |