blob: c3a216aa488a2a49c3bbc1d48e9e3dd311d5fb65 [file] [log] [blame] [view]
Toby Huang5105f812019-08-08 23:47:571# Commit Checklist for Chromium Workflow
2
Toby Huange43e5d682019-10-08 21:26:073Here is a helpful checklist to go through before uploading change lists (CLs) on
Toby Huangacb9beba2020-06-25 20:08:044Gerrit and during the code review process. Gerrit is the code review platform
5for the Chromium project. This checklist is designed to be streamlined. See
Toby Huange43e5d682019-10-08 21:26:076[contributing to Chromium][contributing] for a more thorough reference. The
7intended audience is software engineers who are unfamiliar with contributing to
Toby Huang0a0375c2020-02-21 08:00:288the Chromium project. Feel free to skip steps that are not applicable to the
Toby Huangc9bd85b2020-07-30 22:02:359patchset you're currently uploading.
Toby Huang5105f812019-08-08 23:47:5710
Toby Huangacb9beba2020-06-25 20:08:0411According to the Checklist Manifesto by Atul Gawande, checklists are a marvelous
12tool for ensuring consistent quality in the work you produce. Checklists also
13help you work more efficiently by ensuring you never skip a step or waste brain
14power figuring out the next step to take.
15
Toby Huang5105f812019-08-08 23:47:5716[TOC]
17
Toby Huangacb9beba2020-06-25 20:08:0418## 1. Create a new branch or switch to the correct branch
Toby Huang5105f812019-08-08 23:47:5719
20You should create a new branch before starting any development work. It's
Toby Huang0a0375c2020-02-21 08:00:2821helpful to branch early and to branch often in Git. Use the command
22`git new-branch <branch_name>`. This is equivalent to
Andrew Williamsbbc1a1e2021-07-21 01:51:2223`git checkout -b <branch_name> --track origin/main`.
Toby Huang5105f812019-08-08 23:47:5724
Toby Huang0a0375c2020-02-21 08:00:2825You may also want to set another local branch as the upstream branch. You can do
Toby Huangacb9beba2020-06-25 20:08:0426that with `git checkout -b <branch_name> --track <upstream_branch>`. Do this if
27you want to split your work across multiple CLs, but some CLs have dependencies
Toby Huangd3588732021-04-01 21:47:3428on others. Use `git new-branch --upstream_current <new_branch_name>` to create a
29new branch while setting the current branch as the upstream.
Toby Huang5105f812019-08-08 23:47:5730
Toby Huang5c3c00e2019-10-30 23:29:0531Mark the associated crbug as "started" so that other people know that you have
Toby Huangacb9beba2020-06-25 20:08:0432started working on the bug. Taking this step can avoid duplicated work.
Toby Huang5c3c00e2019-10-30 23:29:0533
Toby Huangacb9beba2020-06-25 20:08:0434If you have already created a branch, don't forget to `git checkout
35<branch_name>` to the correct branch before resuming development work. There's
36few things more frustrating than to finish implementing your ideas or feedback,
37and to spend hours debugging some mysterious bug, only to discover that the bug
38was caused by working on the wrong branch this whole time.
39
40## 2. If there's a local upstream branch, rebase the upstream changes
41
42Suppose you have a downstream branch chained to an upstream branch. If you
43commit changes to the upstream branch, and you want the changes to appear in
44your downstream branch, you need to:
45
46* `git checkout <branch_name>` to the downstream branch.
47* Run `git rebase -i @{u}` to pull the upstream changes into the current
48 branch.
49* Run `git rebase -i @{u}` again to rebase the downstream changes onto the
50 upstream branch.
51
Toby Huang2c40fc52020-08-10 21:44:4052Expect to fix numerous merge conflicts. Use `git rebase --continue` once you're
53done.
54
Toby Huangacb9beba2020-06-25 20:08:0455## 3. Make your changes
Toby Huangba5bbb42019-09-04 23:23:0756
57Do your thing. There's no further advice here about how to write or fix code.
58
Toby Huangacb9beba2020-06-25 20:08:0459## 4. Make sure the code builds correctly
Toby Huang5105f812019-08-08 23:47:5760
61After making your changes, check that common targets build correctly:
62
63* chrome (for Linux, ChromeOS, etc.)
64* unit_tests
65* browser_tests
66
Toby Huangfb638262020-09-21 20:11:0067You can find [instructions here][build-instructions] for building various
68targets.
69
Toby Huang5105f812019-08-08 23:47:5770It's easy to inadvertently break one of the other builds you're not currently
71working on without realizing it. Even though the Commit Queue should catch any
72build errors, checking locally first can save you some time since the CQ Dry Run
Toby Huangacb9beba2020-06-25 20:08:0473can take a while to run, on the order of a few hours sometimes.
Toby Huang5105f812019-08-08 23:47:5774
Toby Huangacb9beba2020-06-25 20:08:0475## 5. Test your changes
Toby Huang5105f812019-08-08 23:47:5776
Toby Huangfb638262020-09-21 20:11:0077Test your changes manually by running the Chrome binary or deploying your
78changes to a test device. If you're testing Chrome for ChromeOS, follow the
79[Simple Chrome][simple-chrome] instructions to deploy your changes to a test
80device. Make sure you hit every code path you changed.
Toby Huang5105f812019-08-08 23:47:5781
Toby Huangd3588732021-04-01 21:47:3482Some testing tips:
Andrew Williamse223ab92021-07-16 23:40:2783* Use `LOG(ERROR) << "debug print statement"` for debugging. You can find
Amanda Lin Dietzd86b8072022-06-29 20:37:3184 the logs in /var/logs/chrome/ on the ChromeOS device. You can add a
85 keyword to your print statement to help find your log statements
86 more quickly.
Toby Huangd3588732021-04-01 21:47:3487* Use GDB for setting breakpoints while debugging.
88
Toby Huangb54e13322020-10-29 18:38:0289Think about testing any edge cases that could break your code. Some common edge
90cases to consider:
91
92* Guest mode
93* Enterprise/EDU/Supervised users
94* Accessibility
95* Official Chrome-branded build (for Googlers)
96
Toby Huangacb9beba2020-06-25 20:08:0497## 6. Write unit or browser tests for any new code
Toby Huang5105f812019-08-08 23:47:5798
99Consider automating any manual testing you did in the previous step.
100
Toby Huangacb9beba2020-06-25 20:08:04101## 7. Ensure the code is formatted nicely
Toby Huang5105f812019-08-08 23:47:57102
103Run `git cl format --js`. The `--js` option also formats JavaScript changes.
104
Toby Huangacb9beba2020-06-25 20:08:04105## 8. Review your changes
Toby Huang5105f812019-08-08 23:47:57106
Toby Huang0a0375c2020-02-21 08:00:28107Use `git diff` to review all of the changes you've made from the previous
108commit. Use `git upstream-diff` to review all of the changes you've made
109from the upstream branch. The output from `git upstream-diff` is what will
110be uploaded to Gerrit.
Toby Huang5105f812019-08-08 23:47:57111
Toby Huangacb9beba2020-06-25 20:08:04112## 9. Stage relevant files for commit
Toby Huang5105f812019-08-08 23:47:57113
Toby Huang0a0375c2020-02-21 08:00:28114Run `git add <path_to_file>` for all of the files you've modified that you want
115to include in the CL. Unlike other version-control systems such as svn, you have
116to specifically `git add` the files you want to commit before calling
117`git commit`.
Toby Huang5105f812019-08-08 23:47:57118
Toby Huangacb9beba2020-06-25 20:08:04119## 10. Commit your changes
Toby Huang5105f812019-08-08 23:47:57120
Toby Huang0a0375c2020-02-21 08:00:28121Run `git commit`. Be sure to write a useful commit message. Here are some
122[tips for writing good commit messages][uploading-a-change-for-review]. A
Toby Huangfb650002020-10-07 19:59:33123shortcut for combining the previous step and this one is `git commit -a -m
Toby Huangacb9beba2020-06-25 20:08:04124<commit_message>`.
Toby Huang5105f812019-08-08 23:47:57125
Toby Huangacb9beba2020-06-25 20:08:04126## 11. Squash your commits
Toby Huangba5bbb42019-09-04 23:23:07127
128If you have many commits on your current branch, and you want to avoid some
Toby Huang0a0375c2020-02-21 08:00:28129nasty commit-by-commit merge conflicts in the next step, consider collecting all
130your changes into one commit. Run `git rebase -i @{u}`. The `@{u}` is a
Andrew Williamsbbc1a1e2021-07-21 01:51:22131short-hand pointer for the upstream branch, which is usually origin/main, but
Toby Huangacb9beba2020-06-25 20:08:04132can also be one of your local branches. After running the `git rebase` command,
133you should see a list of commits, with each commit starting with the word
134"pick". Make sure the first commit says "pick" and change the rest from "pick"
135to "squash". This will squash each commit into the previous commit, which will
136continue until each commit is squashed into the first commit.
Toby Huangba5bbb42019-09-04 23:23:07137
Toby Huangacb9beba2020-06-25 20:08:04138An alternative way to squash your commits into a single commit is to do `git
139commit --amend` in the previous step.
Toby Huang5105f812019-08-08 23:47:57140
Thiago Perrotta6e6630b2022-07-22 07:58:46141Alternatively you can also run `git squash-branch`.
142
Toby Huangacb9beba2020-06-25 20:08:04143## 12. Rebase your local repository
144
145Rebasing is a neat way to sync changes from the remote repository and resolve
146any merge conflict errors on your CL. Run `git rebase-update`. This command
147updates all of your local branches with remote changes that have landed since
148you started development work, which could've been a while ago. It also deletes
149any branches that match the remote repository, such as after the CL associated
150with that branch has been merged. In summary, `git rebase-update` cleans up your
151local branches.
Toby Huang5105f812019-08-08 23:47:57152
Toby Huang0a0375c2020-02-21 08:00:28153You may run into rebase conflicts. Fix them manually before proceeding with
Toby Huangfb650002020-10-07 19:59:33154`git rebase --continue`.
155
156Note that rebasing has the potential to break your build, so you might want to
157try re-building afterwards. You need to run `gclient sync -D` before trying to
158build again after a rebase-update, to update third-party dependencies.
Toby Huang5105f812019-08-08 23:47:57159
Toby Huangacb9beba2020-06-25 20:08:04160## 13. Upload the CL to Gerrit
Toby Huang5105f812019-08-08 23:47:57161
162Run `git cl upload`. Some useful options include:
163
Toby Huangc9bd85b2020-07-30 22:02:35164* `--cq-dry-run` (or `-d`) will set the patchset to do a CQ Dry Run. It is a
165 good idea to run try jobs for each new patchset with significant changes.
Toby Huange43e5d682019-10-08 21:26:07166* `-r <chromium_username>` will add reviewers.
167* `-b <bug_number>` automatically populates the bug reference line of the
Darik Harterd48b03232021-04-07 21:47:04168 commit message. Use `-b None` if there is no relevant crbug.
Amanda Lin Dietzd86b8072022-06-29 20:37:31169* `-x <bug_number>` automatically populates the bug reference line of the
170 commit message and will automatically mark the bug as closed when the
171 CL is submitted and merged.
Toby Huangfb638262020-09-21 20:11:00172* `--edit-description` will let you update the commit message. Using square
173 brackets in the commit message title, like [hashtag], will add a hashtag to
174 your CL. This feature is useful for grouping related CLs together.
Toby Huang5105f812019-08-08 23:47:57175
Toby Huangd3588732021-04-01 21:47:34176Check `git cl issue` to ensure that you are uploading to the correct Gerrit CL.
177If you are uploading a new CL, then the issue number will be none. Uploading
178will automatically create a new CL. Use `git cl issue <issue_number>` to target
179an existing CL for uploading new patches.
180
Toby Huangc9bd85b2020-07-30 22:02:35181To help guide your reviewers, it is also recommended to provide a title for each
182patchset summarizing the changes and indicating whose comments the patchset
183addresses. Running `git cl upload` will upload a new patchset and prompt you for
Daniel Libby12af6b22021-03-31 23:30:39184a brief patchset title. The title defaults to your most recent commit summary
185(the `-T` option will use this without prompting). If you tend to squash all
186your commits into one, try to enter a new summary each time you upload. You can
187also modify the patchset title directly in Gerrit.
Toby Huangc9bd85b2020-07-30 22:02:35188
Toby Huangacb9beba2020-06-25 20:08:04189## 14. Check the CL again in Gerrit
Toby Huang5105f812019-08-08 23:47:57190
191Run `git cl web` to go to the Gerrit URL associated with the current branch.
Toby Huangc9bd85b2020-07-30 22:02:35192Open the latest patchset and verify that all of the uploaded files are correct.
Toby Huange43e5d682019-10-08 21:26:07193Click `Expand All` to check over all of the individual line-by-line changes
Toby Huangacb9beba2020-06-25 20:08:04194again. Basically do a self-review before asking your reviewers for a review.
Toby Huang5105f812019-08-08 23:47:57195
Toby Huangacb9beba2020-06-25 20:08:04196## 15. Make sure all auto-regression tests pass
Toby Huang5105f812019-08-08 23:47:57197
198Click `CQ Dry Run`. Fix any errors because otherwise the CL won't pass the
199commit queue (CQ) checks. Consider waiting for the CQ Dry Run to pass before
200notifying your reviewers, in case the results require major changes in your CL.
201
Thiago Perrotta6e6630b2022-07-22 07:58:46202Alternatively you can run `git cl try`.
203
Toby Huangacb9beba2020-06-25 20:08:04204## 16. Add reviewers to review your code
Toby Huang5105f812019-08-08 23:47:57205
206Click `Find Owners` or run `git cl owners` to find file owners to review your
Daniel Murphya32d1cd2021-07-22 16:25:52207code and instruct them about which parts you want them to focus on. Prefer
208owners who are more specific to files you are modifying, as they usually
209have the best domain knowledge (i.e. prefer `//chrome/foo/bar/OWNERS` over
210`//chrome/OWNERS`). Next, add anyone else you think should review your code. The
211blame functionality in Code Search is a good way to identify reviewers who may
212be familiar with the parts of code your CL touches. For your CL to land, you
213need an approval from an owner for each file you've changed, unless you are an
214owner of some files, in which case you don't need separate owner approval for
215those files.
216
217You are expected to wait for all actively participating reviewers to CR+1 the
218change before submitting (CQ+2), even if your CL already has all required owners
219reviews. Other than preventing confusion and mistakes, this expectation exists
220because:
David Sanders1a95e9c2022-01-17 03:14:392211. Participating reviewers are
222 [helping you write sustainable code][sustainable-code], and letting them sign
223 off is respectful of their efforts.
Daniel Murphya32d1cd2021-07-22 16:25:522241. The owners system is not perfect, and sometimes you will need an owner who
225 *can* approve the whole change, but will delegate approval of pieces to
226 other, more knowledgeable owners.
227
228If this expectation needs to be broken, then the reason should be justified in a
229comment, and appropriate extra care may be appropriate (e.g. getting a
230post-submit review, monitoring for failing or flaky tests, reverting if any
231problems occur, etc).
Toby Huang5105f812019-08-08 23:47:57232
Howard Wolosky7c06f7fa2021-01-20 18:37:35233## 17. Start Your Review
234
235Click on the `Start Review` button to begin the actual review process. Until
236you press this button, nobody will look at your change. Once pressed, you'll
237have the opportunity to include an additional message in the notification sent
238to your reviewers.
239
240## 18. Implement feedback from your reviewers
Toby Huang5105f812019-08-08 23:47:57241
242Then go through this commit checklist again. Reply to all comments from the
Daniel Murphyee8b6162021-07-14 23:48:19243reviewers on Gerrit and mark all resolved issues as resolved. To see all
244unresolved comments, click on the "Comments" tab in Gerrit. Other than freeform
245interaction on the comments (using `Reply` or `Quote`), here are common
246conventions:
247* Clicking `Done` on the comment will comment "Done" and resolve this comment.
248 This usually is used in response to a requested change by the reviewer, and
249 tells the reviewer that you have made the change that they requested.
250* Clicking `Ack` on the comment will comment "Ack" (short for "Acknowledged")
251 and resolve this comment. This usually is used in response to a non-actionable
252 comment by the reviewer, and tells the reviewer that you understand.
253
254Finally, click `Reply` on the CL to ensure that your reviewers receive a
255notification. Doing this signals that your CL is ready for review again, since
256the assumption is that your CL is not ready for review until you hit reply.
Toby Huang5105f812019-08-08 23:47:57257
Daniel Murphya32d1cd2021-07-22 16:25:52258To ensure a fast, productive, and respectful review, please follow the
259guidelines in [Respectful Changes][respectful-changes].
260
Toby Huangfb638262020-09-21 20:11:00261If your change is simple and you feel confident that your reviewer will approve
262your CL on the next iteration, you can set Auto-Submit +1. The CL will proceed
263to the next step automatically after approval. This feature is useful if your
264reviewer is in a different time zone and you want to land the CL sooner. Setting
265this flag also puts the onus on your reviewer to land the CL.
266
Howard Wolosky7c06f7fa2021-01-20 18:37:35267## 19. Land your CL
Toby Huang5105f812019-08-08 23:47:57268
Erik Staab3d5a4b992024-03-20 16:33:57269To meet the minimum requirements to land your changes you must have:
270* Obtained a Looks Good To Me (LGTM), which is reflected by a
271 Code-Review+1 in Gerrit
272 * from at least one owner for each file, excluding files you are an owner of
273 * from two committers, or one committer if you are also a committer
274* Resolved all code review comments
275
276As mentioned above, you are generally expected to wait for all of your reviewers
277to approve your changes as well, even if you already have OWNERS approval. Don't
278use `chrome/OWNERS` as a blanket stamp if your CL makes significant changes to
279subsystems. Click `Submit to CQ` (Commit-Queue +2) to both try your change in
280the commit queue (CQ) and automatically land it if successful.
Toby Huang5105f812019-08-08 23:47:57281
Thiago Perrotta6e6630b2022-07-22 07:58:46282Alternatively you can run `git cl set-commit`.
283
Toby Huangfb650002020-10-07 19:59:33284Just because your CL made it through the CQ doesn't mean you're in the clear
285yet. There might be internal non-public try job failures, or bugs that went
286unnoticed during the code review process. Consider monitoring the
287[Chromium tree][chromium-tree] for about a day after your CL lands. If
288the Sheriff or anyone else brings any failures to your attention, revert the CL
289first and ask questions later. Gerrit can automatically generate revert CLs.
290
Howard Wolosky7c06f7fa2021-01-20 18:37:35291## 20. Cleanup
Toby Huang5c3c00e2019-10-30 23:29:05292
Toby Huang5105f812019-08-08 23:47:57293After your CL is landed, you can use `git rebase-update` or `git cl archive` to
Toby Huang5c3c00e2019-10-30 23:29:05294clean up your local branches. These commands will automatically delete merged
Juha Vainioa24c5b782021-09-27 10:18:13295branches. Please mark the associated crbug as "fixed".
Toby Huang5105f812019-08-08 23:47:57296
297[//]: # (the reference link section should be alphabetically sorted)
John Palmer046f9872021-05-24 01:24:56298[build-instructions]: https://chromium.googlesource.com/chromium/src.git/+/main/docs/#Checking-Out-and-Building
Toby Huangfb650002020-10-07 19:59:33299[chromium-tree]: https://ci.chromium.org/p/chromium/g/main/console
Toby Huang5105f812019-08-08 23:47:57300[contributing]: contributing.md
Daniel Murphya32d1cd2021-07-22 16:25:52301[respectful-changes]: cl_respect.md
Amanda Lin Dietzd86b8072022-06-29 20:37:31302[simple-chrome]: https://chromium.googlesource.com/chromiumos/docs/+/HEAD/simple_chrome_workflow.md
David Sanders1a95e9c2022-01-17 03:14:39303[sustainable-code]: cr_respect.md
Amanda Lin Dietzd86b8072022-06-29 20:37:31304[uploading-a-change-for-review]: contributing.md#Uploading-a-change-for-review