blob: e2c5b597fd3abe8d944898009dfdff4b9e5fcc40 [file] [log] [blame] [view]
Abdul Syede21aa1b2017-07-06 17:13:061# Merge Request Process
2
3[TOC]
4
5## tl;dr
6
Harry Souders782e8b2c2024-02-05 06:24:507* 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)
11 to understand which branches are active and what merges are acceptable for
12 each 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
Harry Souders8b7e12e82025-01-13 23:03:0915* Use Chromium Issue Tracker's [project queries](#monitoring-merge-requests) to
16 track your approved merges as well as your pending requests
Harry Souders782e8b2c2024-02-05 06:24:5017* 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
Harry Souders782e8b2c2024-02-05 06:24:5029fix a web platform regression, address a crash or patch a security
30vulnerability.
Abdul Syede21aa1b2017-07-06 17:13:0631
Alex Mineerc5b63022021-08-26 21:36:0532To ensure we make the right decisions, release managers leverage a merge review
33process to evaluate each request. They'll ask questions about the reason you
34would like to merge a change and the risk of the merge itself, and you'll work
Harry Souders782e8b2c2024-02-05 06:24:5035together to make a judgement call on whether or not the merge should be approved
36or rejected.
Abdul Syede21aa1b2017-07-06 17:13:0637
Alex Mineerc5b63022021-08-26 21:36:0538Generally, merges follow these high-level steps:
Abdul Syede21aa1b2017-07-06 17:13:0639
Harry Souders782e8b2c2024-02-05 06:24:5040* Developers update bug with relevant details and request a merge by updating
41 the *Merge-Request* field with the requested milestones, then wait for
42 review.
43* Release managers and automation review and approve, reject, or ask questions
44 about the merge within two business days.
45* Developers wait for review and, if approved, land the merge ASAP.
Abdul Syede21aa1b2017-07-06 17:13:0646
Alex Mineerc5b63022021-08-26 21:36:0547For details on each step, see below.
Abdul Syede21aa1b2017-07-06 17:13:0648
Amy Ressler373f77722024-02-03 00:39:4749**NOTE:** Because security issues (identified with *Type=Vulnerability*) follow
Harry Souders782e8b2c2024-02-05 06:24:5050a more complex flow, you may simply mark security issues as *Fixed* in the Issue
51Tracker and [automation](#security-merge-triage) will handle the remainder of
52the merge request process flow for you; simply process the merge if it is
53requested and approved.
Abdul Syede21aa1b2017-07-06 17:13:0654
Alex Mineerc5b63022021-08-26 21:36:0555## Requesting a merge
Abdul Syede21aa1b2017-07-06 17:13:0656
Alex Mineerc5b63022021-08-26 21:36:0557### Verifying eligibility and safety
Abdul Syede21aa1b2017-07-06 17:13:0658
Alex Mineerc5b63022021-08-26 21:36:0559Before requesting a merge, first ensure your change is a good merge candidate:
Abdul Syede21aa1b2017-07-06 17:13:0660
Harry Souders782e8b2c2024-02-05 06:24:5061* Ensure it meets the merge criteria (via
62 [Chromium Dash](https://chromiumdash.appspot.com/branches)) of the
63 branch(es) you'd like to merge to; merge criteria become more strict the
64 older the branch is, more details on criteria
65 [below](#merge-criteria-phases)
66* Verify merging the change to an older branch would be safe, e.g. unlikely to
67 introduce new regressions, no major merge conflicts, automated test coverage
68 present, etc; chat with your TL for input if you're not sure
69* Confirm your change fixes the issue at hand, preferably by testing on and
70 monitoring the canary channel for 24 hours post-release (see
71 [Chromium Dash](https://chromiumdash.appspot.com/commits) to determine if
72 your change has shipped)
Abdul Syede21aa1b2017-07-06 17:13:0673
Harry Souders782e8b2c2024-02-05 06:24:5074 * You may skip this step if a release manager or security team member has
75 told you that the merge is urgent, e.g. is actively blocking a release
Alex Mineer40d33c32018-03-15 23:45:1476
Amy Ressler373f77722024-02-03 00:39:4777### Updating Chromium Issue Tracker
Abdul Syede21aa1b2017-07-06 17:13:0678
Harry Souders782e8b2c2024-02-05 06:24:5079Next, update the bug (generally the bug being fixed by the merge) with the
80following information present and accurate:
Abdul Syede21aa1b2017-07-06 17:13:0681
Harry Souders782e8b2c2024-02-05 06:24:5082* Title and description clearly describing the bug being fixed
83* Priority (*Priority*), OS (*OS*) and target milestone(s) (*Milestone*)
84 fields are set
Philip Rogers1d50df092024-07-22 13:27:2885 * Consider all available data when setting the priority, such as existing
86 metrics for usage of a broken feature, to ensure important merges are
87 not missed. Consider collecting new data, such as by landing new metrics
88 and estimating severity with pre-stable data. For Web Platform changes,
Harry Souders8b7e12e82025-01-13 23:03:0989 [compat
90 tools](https://www.chromium.org/blink/platform-predictability/compat-tools/)
Philip Rogers1d50df092024-07-22 13:27:2891 such as
92 [UseCounters](https://www.chromium.org/blink/platform-predictability/compat-tools/#usecounter)
Harry Souders8b7e12e82025-01-13 23:03:0993 , [Cluster
94 Telemetry](https://www.chromium.org/blink/platform-predictability/compat-tools/#on-demand-crawl)
Philip Rogers1d50df092024-07-22 13:27:2895 , and
96 [HTTPArchive](https://www.chromium.org/blink/platform-predictability/compat-tools/#the-http-archive)
97 may be useful.
98
Harry Souders782e8b2c2024-02-05 06:24:5099* Owner, generally the person requesting / performing the merge
100* [Release block label](./release_blockers.md) if applicable (*ReleaseBlock*
101 field*)
102* Issue status:
Abdul Syede21aa1b2017-07-06 17:13:06103
Harry Souders782e8b2c2024-02-05 06:24:50104 * Fixed: You're confident the issue is fixed on main, e.g. you've locally
105 built and tested the issue, no additional crash reports are generated
106 after the fix was released, etc (most issues)
107 * In Progress (Accepted): Diagnostic merges only, e.g. to merge code to
108 track down the root cause of an issue that only exists on branch
Abdul Syede21aa1b2017-07-06 17:13:06109
Harry Souders782e8b2c2024-02-05 06:24:50110### Setting the Merge-Request field
Abdul Syede21aa1b2017-07-06 17:13:06111
Amy Ressler373f77722024-02-03 00:39:47112Once you've verified all the above, you're ready to request a merge! Simply
Harry Souders782e8b2c2024-02-05 06:24:50113update the issue's *Merge-Request* field with the milestone(s) you'd like to
Harry Souders1907ad72025-01-22 17:16:03114merge to. Within the next ~15 minutes, automation will create a new Merge
115Request issue, link it to the original (parent) issue, copy relevant metadata,
116and assign the issue to you.
117
118### Submitting the Merge Request
119
120Follow the instruction in the new Merge Request issue. You will be asked to
121update a number of custom fields (found in the issue's sidebar) and provide a
122rationale for the merge request. After you have updated and verified the
123request, you can submit the request for review by assigning the issue to
124*[email protected]*.
Abdul Syede21aa1b2017-07-06 17:13:06125
Alex Mineerc5b63022021-08-26 21:36:05126## Monitoring merge requests
Abdul Syede21aa1b2017-07-06 17:13:06127
Harry Souders1907ad72025-01-22 17:16:03128After assigning the issue to *[email protected]*, automation will evaluate
129your request and either approve it or pass it along to a release manager for
130manual evaluation; see [here](#merge-request-triage) to learn more about this
131automation. If manual review is required, release managers strive to answer all
132merge requests within two business days, but extenuating circumstances may cause
133delays.
Abdul Syede21aa1b2017-07-06 17:13:06134
Alex Mineerc5b63022021-08-26 21:36:05135At this point, following along via bug comments sent by email will always keep
Harry Souders782e8b2c2024-02-05 06:24:50136you in the loop, but you can also use the following queries in the Issue Tracker
137to track your merges:
Abdul Syede21aa1b2017-07-06 17:13:06138
Harry Souders8b7e12e82025-01-13 23:03:09139* [Approved and TBD merges](https://issues.chromium.org/issues?q=assignee:me%20customfield1223087:(Approved%20%7C%20TBD)):
Harry Souders782e8b2c2024-02-05 06:24:50140 Merges that require your follow-up, either by landing the relevant merge (if
141 approved) or determining whether or not a merge is actually required and if
142 so, requesting it (if TBD)
Harry Souders1907ad72025-01-22 17:16:03143* [Pending merges](https://g-issues.chromium.org/issues?q=assignee:me%20customfield1223087:Pending):
144 Merges that are pending review by automation and need additional action from
145 you. Frequently, issues remain in the state because developers have failed
146 to update the merge request issue with all required information, or the
147 request was not assigned to **[email protected]**. Please follow the
148 instructions in the issue to complete the merge request, and reach out to
149 a release manager if you have any questions.
Harry Souders8b7e12e82025-01-13 23:03:09150* [Requested
151 merges](https://issues.chromium.org/issues?q=assignee:me%20(-customfield1223134:none%20%7C%20customfield1223087:Review)):
Harry Souders782e8b2c2024-02-05 06:24:50152 Merges that are waiting for input from release managers or automation; feel
153 free to ping bugs that sit in this queue for two business days (assuming you
154 verified that the change was already deployed to canary ahead of requesting
155 a merge)
Harry Souders8b7e12e82025-01-13 23:03:09156* [Rejected and NA merges](https://issues.chromium.org/issues?q=assignee:me%20customfield1223087:(Rejected%20%7C%20NA)):
Harry Souders782e8b2c2024-02-05 06:24:50157 Merges that were either rejected by release managers, or not applicable to
158 be merged; generally, no action is needed for these items unless you
159 disagree with a merge's rejection and wish to escalate
Harry Souders8b7e12e82025-01-13 23:03:09160* [All merges](https://issues.chromium.org/issues?q=assignee:me%20(-customfield1223087:none%20%7C%20-customfield1223134:none)):
Harry Souders782e8b2c2024-02-05 06:24:50161 Includes every possible merge state, useful when wanting to find an item you
162 considered for merging but can't recall the state it was last in.
Abdul Syede21aa1b2017-07-06 17:13:06163
Alex Mineerc5b63022021-08-26 21:36:05164For a description of each label used to track the merge process, see the
165appendix [below](#merge-states-and-labels).
Abdul Syede21aa1b2017-07-06 17:13:06166
Alex Mineerc5b63022021-08-26 21:36:05167## Landing an approved merge
Abdul Syede21aa1b2017-07-06 17:13:06168
Harry Souders782e8b2c2024-02-05 06:24:50169Once your merge has been approved for a given milestone (via the release manager
170or automation updating the *Merge* field with *Approved-###*), you have two
171options to land the merge:
Abdul Syede21aa1b2017-07-06 17:13:06172
Harry Souders782e8b2c2024-02-05 06:24:50173* Gerrit UI, easiest for clean cherry-picks or those requiring only minor
174 changes
175* git, for more complex cherry-picks and / or when local verification may be
176 beneficial
Alan Cuttere338ed92021-05-06 04:57:19177
Alex Mineerc5b63022021-08-26 21:36:05178Regardless of which method you choose, please ensure you land your cherry-pick
179ASAP so that it can be included in the next release built from the branch; if
180you don't merge your cherry-pick soon after approval, it will eventually be
181rejected for merge.
Abdul Syede21aa1b2017-07-06 17:13:06182
Harry Souders8b7e12e82025-01-13 23:03:09183**NOTE:** Ensure you link to the bug that has merge approval for the relevant
Ben Masona48d7962025-02-13 16:06:47184milestone. Not linking to a bug that has approval can cause delay to your CL
185landing. If the merge request is for a single change, add `Fixed: <bug number>`
186to the change description. If the merge request is for multiple changes, add
187`Fixed: <bug number>` to the final change description, `Bug: <bug number>`
188to all other change descriptions.
Ben Masone8424ee22024-12-04 11:23:43189
Harry Souders782e8b2c2024-02-05 06:24:50190Once the cherry-pick has landed, a bot will update the *Merge* field with
Ben Masone0b04992025-03-03 19:07:35191*Merged-###* label. If `Fixed:` was used, the bug will be closed.
Ben Wagner96babd4c2022-08-26 16:53:09192
Alex Mineerc5b63022021-08-26 21:36:05193### Using Gerrit UI
Abdul Syede21aa1b2017-07-06 17:13:06194
David Benjamin58230aec2022-02-14 22:35:23195Select the "..." button in the Gerrit UI, then choose "Cherry Pick". When
Harry Souders782e8b2c2024-02-05 06:24:50196prompted for a branch, enter *refs/branch-heads/####*, where #### corresponds to
197the release branch you are merging to (available on
mlcuieaa3e3952022-02-14 22:52:06198[Chromium Dash](https://chromiumdash.appspot.com/branches) in the "Chromium"
199column).
Abdul Syede21aa1b2017-07-06 17:13:06200
Harry Souders782e8b2c2024-02-05 06:24:50201Once the cherry-pick CL is prepared, you can bypass code review (but not OWNERS
202approval) within 14 days of the original change by adding the Rubber Stamper bot
203(rubber-stamper@appspot.gserviceaccount.com) as a reviewer. If the CL meets the
K. Moon216a9a1192022-08-26 06:02:36204[Rubber Stamper criteria](https://chromium.googlesource.com/infra/infra/+/refs/heads/main/go/src/infra/appengine/rubber-stamper/README.md),
205the bot will vote *Bot-Commit+1* to bypass code review. If the CL is marked
206*Auto-Submit+1*, the bot will also submit the CL to the CQ on your behalf.
Ian Barkley-Yeung3af975e2021-04-06 19:00:35207
Alex Mineerc5b63022021-08-26 21:36:05208### Using git
209
210The commands below should set up your environment to be able to successfully
211upload a cherry-pick to a release branch, where *####* corresponds to the
212release branch you are merging to (available on
mlcuieaa3e3952022-02-14 22:52:06213[Chromium Dash](https://chromiumdash.appspot.com/branches) in the "Chromium"
214column):
Alex Mineerc5b63022021-08-26 21:36:05215
216```
217$ gclient sync --with_branch_heads
218$ git fetch
219$ git checkout -b BRANCH_NAME refs/remotes/branch-heads/####
220$ git cl upstream branch-heads/####
221$ git cherry-pick -x COMMIT_HASH_MAIN
Yoav Weiss42be0432022-11-26 19:04:07222$ gclient sync
Alex Mineerc5b63022021-08-26 21:36:05223```
224
225From here, your environment should be ready to adjust the change as required;
226use ninja to build and test your changes, and when ready upload for review:
227
228```
229$ git cl upload
230```
231
232**Adjust the change description** to omit the "Change-Id: ..." line from
Harry Souders782e8b2c2024-02-05 06:24:50233original patch, otherwise you may experience issues when uploading the change to
234Gerrit. Once complete, use Gerrit to initiate review and approval of the merge
235as TBR has been discontinued.
Alex Mineerc5b63022021-08-26 21:36:05236
Harry Souders782e8b2c2024-02-05 06:24:50237Other tips & tricks when merging with git via release branches: * Consider using
238multiple working directories when creating the release branch * Editing the
239change description to denote this is a merge (e.g. "Merge to release branch" at
240the top) will help reviewers distinguish between the cherry-pick and the
241original change
Alex Mineerc5b63022021-08-26 21:36:05242
243## Merge automation
244
Amy Ressler373f77722024-02-03 00:39:47245The Chrome team has built automation via
246[Blintz](https://www.chromium.org/issue-tracking/autotriage), formerly known as
247Sheriffbot, to assist in several merge flows: security merge triage, general
248merge request triage, and preventing missed merges.
Alex Mineerc5b63022021-08-26 21:36:05249
250### Security merge triage
251
252Given the additional complexity inherent in security merges, the security team
253has built custom automation to handle this flow end to end; simply mark any
Harry Souders1907ad72025-01-22 17:16:03254security issue as *Fixed* and Blintz will evaluate applicable milestones,
Alex Mineerc5b63022021-08-26 21:36:05255determine if merges are required and automatically request them if need be.
256
257### Merge request triage
258
Harry Souders1907ad72025-01-22 17:16:03259To reduce release manager toil, Blintz performs the first pass review of all
Harry Souders782e8b2c2024-02-05 06:24:50260merge requests; it may auto-approve the issue if it can detect the issue meets
261the right criteria for the current merge phase (e.g. a ReleaseBlock-Dev issue
Harry Souders1907ad72025-01-22 17:16:03262requesting a merge before beta promotion). If it cannot decide, it will pass the
263issue to a release manager for manual review.
Alex Mineerc5b63022021-08-26 21:36:05264
Harry Souders1907ad72025-01-22 17:16:03265Blintz only takes action on merge requests when the merge request issue (not the
266original issue) is assigned to *merges@chromium.org*.
Alex Mineerc5b63022021-08-26 21:36:05267
268### Preventing missed merges
269
Harry Souders8b7e12e82025-01-13 23:03:09270To avoid the situation where a critical issue is present on a release branch
Harry Souders1907ad72025-01-22 17:16:03271but the fix isn't merged, Blintz evaluates all release-blocking issues
Harry Souders8b7e12e82025-01-13 23:03:09272targeting a milestone that has already branched and updates the *Merge* field
273with *TBD-##* if the issue was marked as fixed after branch day but hasn't been
Amy Ressler373f77722024-02-03 00:39:47274merged. When this occurs, developers should evaluate the issue and either
275request a merge if required (e.g. the fix did miss the release branch point) by
Harry Souders782e8b2c2024-02-05 06:24:50276updating the *Merge-Request* field, or update the *Merge* field with *NA-###*
277(e.g. the fix is present in the release branch already or the merge is
Amy Ressler373f77722024-02-03 00:39:47278unnecessary for other reasons).
Alex Mineerc5b63022021-08-26 21:36:05279
280## Appendix
281
282### Merge criteria phases
283
284The table below describes the different phases that each milestone progresses
Harry Souders782e8b2c2024-02-05 06:24:50285through during its release cycle; this data is available via the Chromium Dash
286[front-end](https://chromiumdash.appspot.com/branches) and
Alex Mineerc5b63022021-08-26 21:36:05287[API](https://chromiumdash.appspot.com/fetch_milestones).
288
Harry Souders8b7e12e82025-01-13 23:03:09289| Branch Phase | Period Begins | Period Ends | Acceptable Merges Include Fixes For: |
290| --- | --- | --- | --- |
291| beta | M(X) Branch | 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, emergency string issues (.GRD changes) |
292| stable | M(X) Stable Cut | 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 |
293| 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:05294
295### Merge states and labels
296
Harry Souders782e8b2c2024-02-05 06:24:50297The table below describes the different merge states applied via a bug's
Harry Souders1907ad72025-01-22 17:16:03298metadata fields. All merge states (except *Pending*) follow the form
299*[State]-###*, where ### corresponds to the applicable milestone. If multiple
300merges are required, these labels may appear multiple times on the same bug in
301different states.
Alex Mineerc5b63022021-08-26 21:36:05302
Harry Souders8b7e12e82025-01-13 23:03:09303| Field | Value | Step Owner | Next Steps |
304| --- | --- | --- | --- |
305| Merge-Request | ### | Release manager | Automation will review and either approve / reject directly, or pass the review to a release manager for manual evaluation |
Harry Souders1907ad72025-01-22 17:16:03306| Merge | Pending | Issue owner | Issue owner should follow the instructions in the merge request and assign it to *[email protected]* to begin review |
Harry Souders8b7e12e82025-01-13 23:03:09307| Merge | Review-### | Release manager | Release manager will evaluate and either approve, reject, or request additional information within two business days |
308| Merge | Approved-### | Issue owner | Issue owner should cherry-pick the fix to the appropriate release branch ASAP |
309| Merge | Merged-### | None | N/A; merge has already been landed, no further work required for given milestone |
310| Merge | Rejected-### | Issue owner | Issue owner should re-request a merge to escalate if they feel the merge was erroneously rejected and should be re-evaluated |
311| Merge | TBD-### | Issue owner | Issue owner should evaluate if a merge is required, then remove *TBD-##* and replace it with *NA-##* (if no merge needed) or re-request a merge (if needed) |
312| Merge | NA-### | None | N/A; merge is not required to the relevant milestone, no further work required for given milestone |