blob: ecd3da8e7e88446e1ec864938eb10e618e26f715 [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
Toby Huangd3588732021-04-01 21:47:3484 the logs in /var/logs/chrome/ on the ChromeOS device.
85* Use GDB for setting breakpoints while debugging.
86
Toby Huangb54e13322020-10-29 18:38:0287Think about testing any edge cases that could break your code. Some common edge
88cases to consider:
89
90* Guest mode
91* Enterprise/EDU/Supervised users
92* Accessibility
93* Official Chrome-branded build (for Googlers)
94
Toby Huangacb9beba2020-06-25 20:08:0495## 6. Write unit or browser tests for any new code
Toby Huang5105f812019-08-08 23:47:5796
97Consider automating any manual testing you did in the previous step.
98
Toby Huangacb9beba2020-06-25 20:08:0499## 7. Ensure the code is formatted nicely
Toby Huang5105f812019-08-08 23:47:57100
101Run `git cl format --js`. The `--js` option also formats JavaScript changes.
102
Toby Huangacb9beba2020-06-25 20:08:04103## 8. Review your changes
Toby Huang5105f812019-08-08 23:47:57104
Toby Huang0a0375c2020-02-21 08:00:28105Use `git diff` to review all of the changes you've made from the previous
106commit. Use `git upstream-diff` to review all of the changes you've made
107from the upstream branch. The output from `git upstream-diff` is what will
108be uploaded to Gerrit.
Toby Huang5105f812019-08-08 23:47:57109
Toby Huangacb9beba2020-06-25 20:08:04110## 9. Stage relevant files for commit
Toby Huang5105f812019-08-08 23:47:57111
Toby Huang0a0375c2020-02-21 08:00:28112Run `git add <path_to_file>` for all of the files you've modified that you want
113to include in the CL. Unlike other version-control systems such as svn, you have
114to specifically `git add` the files you want to commit before calling
115`git commit`.
Toby Huang5105f812019-08-08 23:47:57116
Toby Huangacb9beba2020-06-25 20:08:04117## 10. Commit your changes
Toby Huang5105f812019-08-08 23:47:57118
Toby Huang0a0375c2020-02-21 08:00:28119Run `git commit`. Be sure to write a useful commit message. Here are some
120[tips for writing good commit messages][uploading-a-change-for-review]. A
Toby Huangfb650002020-10-07 19:59:33121shortcut for combining the previous step and this one is `git commit -a -m
Toby Huangacb9beba2020-06-25 20:08:04122<commit_message>`.
Toby Huang5105f812019-08-08 23:47:57123
Toby Huangacb9beba2020-06-25 20:08:04124## 11. Squash your commits
Toby Huangba5bbb42019-09-04 23:23:07125
126If you have many commits on your current branch, and you want to avoid some
Toby Huang0a0375c2020-02-21 08:00:28127nasty commit-by-commit merge conflicts in the next step, consider collecting all
128your changes into one commit. Run `git rebase -i @{u}`. The `@{u}` is a
Andrew Williamsbbc1a1e2021-07-21 01:51:22129short-hand pointer for the upstream branch, which is usually origin/main, but
Toby Huangacb9beba2020-06-25 20:08:04130can also be one of your local branches. After running the `git rebase` command,
131you should see a list of commits, with each commit starting with the word
132"pick". Make sure the first commit says "pick" and change the rest from "pick"
133to "squash". This will squash each commit into the previous commit, which will
134continue until each commit is squashed into the first commit.
Toby Huangba5bbb42019-09-04 23:23:07135
Toby Huangacb9beba2020-06-25 20:08:04136An alternative way to squash your commits into a single commit is to do `git
137commit --amend` in the previous step.
Toby Huang5105f812019-08-08 23:47:57138
Toby Huangacb9beba2020-06-25 20:08:04139## 12. Rebase your local repository
140
141Rebasing is a neat way to sync changes from the remote repository and resolve
142any merge conflict errors on your CL. Run `git rebase-update`. This command
143updates all of your local branches with remote changes that have landed since
144you started development work, which could've been a while ago. It also deletes
145any branches that match the remote repository, such as after the CL associated
146with that branch has been merged. In summary, `git rebase-update` cleans up your
147local branches.
Toby Huang5105f812019-08-08 23:47:57148
Toby Huang0a0375c2020-02-21 08:00:28149You may run into rebase conflicts. Fix them manually before proceeding with
Toby Huangfb650002020-10-07 19:59:33150`git rebase --continue`.
151
152Note that rebasing has the potential to break your build, so you might want to
153try re-building afterwards. You need to run `gclient sync -D` before trying to
154build again after a rebase-update, to update third-party dependencies.
Toby Huang5105f812019-08-08 23:47:57155
Toby Huangacb9beba2020-06-25 20:08:04156## 13. Upload the CL to Gerrit
Toby Huang5105f812019-08-08 23:47:57157
158Run `git cl upload`. Some useful options include:
159
Toby Huangc9bd85b2020-07-30 22:02:35160* `--cq-dry-run` (or `-d`) will set the patchset to do a CQ Dry Run. It is a
161 good idea to run try jobs for each new patchset with significant changes.
Toby Huange43e5d682019-10-08 21:26:07162* `-r <chromium_username>` will add reviewers.
163* `-b <bug_number>` automatically populates the bug reference line of the
Darik Harterd48b03232021-04-07 21:47:04164 commit message. Use `-b None` if there is no relevant crbug.
Toby Huangfb638262020-09-21 20:11:00165* `--edit-description` will let you update the commit message. Using square
166 brackets in the commit message title, like [hashtag], will add a hashtag to
167 your CL. This feature is useful for grouping related CLs together.
Toby Huang5105f812019-08-08 23:47:57168
Toby Huangd3588732021-04-01 21:47:34169Check `git cl issue` to ensure that you are uploading to the correct Gerrit CL.
170If you are uploading a new CL, then the issue number will be none. Uploading
171will automatically create a new CL. Use `git cl issue <issue_number>` to target
172an existing CL for uploading new patches.
173
Toby Huangc9bd85b2020-07-30 22:02:35174To help guide your reviewers, it is also recommended to provide a title for each
175patchset summarizing the changes and indicating whose comments the patchset
176addresses. Running `git cl upload` will upload a new patchset and prompt you for
Daniel Libby12af6b22021-03-31 23:30:39177a brief patchset title. The title defaults to your most recent commit summary
178(the `-T` option will use this without prompting). If you tend to squash all
179your commits into one, try to enter a new summary each time you upload. You can
180also modify the patchset title directly in Gerrit.
Toby Huangc9bd85b2020-07-30 22:02:35181
Toby Huangacb9beba2020-06-25 20:08:04182## 14. Check the CL again in Gerrit
Toby Huang5105f812019-08-08 23:47:57183
184Run `git cl web` to go to the Gerrit URL associated with the current branch.
Toby Huangc9bd85b2020-07-30 22:02:35185Open the latest patchset and verify that all of the uploaded files are correct.
Toby Huange43e5d682019-10-08 21:26:07186Click `Expand All` to check over all of the individual line-by-line changes
Toby Huangacb9beba2020-06-25 20:08:04187again. Basically do a self-review before asking your reviewers for a review.
Toby Huang5105f812019-08-08 23:47:57188
Toby Huangacb9beba2020-06-25 20:08:04189## 15. Make sure all auto-regression tests pass
Toby Huang5105f812019-08-08 23:47:57190
191Click `CQ Dry Run`. Fix any errors because otherwise the CL won't pass the
192commit queue (CQ) checks. Consider waiting for the CQ Dry Run to pass before
193notifying your reviewers, in case the results require major changes in your CL.
194
Toby Huangacb9beba2020-06-25 20:08:04195## 16. Add reviewers to review your code
Toby Huang5105f812019-08-08 23:47:57196
197Click `Find Owners` or run `git cl owners` to find file owners to review your
198code and instruct them about which parts you want them to focus on. Add anyone
Toby Huange43e5d682019-10-08 21:26:07199else you think should review your code. The blame functionality in Code Search
200is a good way to identify reviewers who may be familiar with the parts of code
201your CL touches. For your CL to land, you need an approval from an owner for
202each file you've changed, unless you are an owner of some files, in which case
203you don't need separate owner approval for those files.
Toby Huang5105f812019-08-08 23:47:57204
Howard Wolosky7c06f7fa2021-01-20 18:37:35205## 17. Start Your Review
206
207Click on the `Start Review` button to begin the actual review process. Until
208you press this button, nobody will look at your change. Once pressed, you'll
209have the opportunity to include an additional message in the notification sent
210to your reviewers.
211
212## 18. Implement feedback from your reviewers
Toby Huang5105f812019-08-08 23:47:57213
214Then go through this commit checklist again. Reply to all comments from the
Daniel Murphyee8b6162021-07-14 23:48:19215reviewers on Gerrit and mark all resolved issues as resolved. To see all
216unresolved comments, click on the "Comments" tab in Gerrit. Other than freeform
217interaction on the comments (using `Reply` or `Quote`), here are common
218conventions:
219* Clicking `Done` on the comment will comment "Done" and resolve this comment.
220 This usually is used in response to a requested change by the reviewer, and
221 tells the reviewer that you have made the change that they requested.
222* Clicking `Ack` on the comment will comment "Ack" (short for "Acknowledged")
223 and resolve this comment. This usually is used in response to a non-actionable
224 comment by the reviewer, and tells the reviewer that you understand.
225
226Finally, click `Reply` on the CL to ensure that your reviewers receive a
227notification. Doing this signals that your CL is ready for review again, since
228the assumption is that your CL is not ready for review until you hit reply.
Toby Huang5105f812019-08-08 23:47:57229
Toby Huangfb638262020-09-21 20:11:00230If your change is simple and you feel confident that your reviewer will approve
231your CL on the next iteration, you can set Auto-Submit +1. The CL will proceed
232to the next step automatically after approval. This feature is useful if your
233reviewer is in a different time zone and you want to land the CL sooner. Setting
234this flag also puts the onus on your reviewer to land the CL.
235
Howard Wolosky7c06f7fa2021-01-20 18:37:35236## 19. Land your CL
Toby Huang5105f812019-08-08 23:47:57237
238Once you have obtained a Looks Good To Me (LGTM), which is reflected by a
239Code-Review+1 in Gerrit, from at least one owner for each file, then you have
240the minimum prerequisite to land your changes. It may be helpful to wait for all
241of your reviewers to approve your changes as well, even if they're not owners.
Toby Huangfb638262020-09-21 20:11:00242Don't use `chrome/OWNERS` as a blanket stamp if your CL makes significant
243changes to subsystems. Click `Submit to CQ` to try your change in the commit
244queue (CQ), which will land it if successful.
Toby Huang5105f812019-08-08 23:47:57245
Toby Huangfb650002020-10-07 19:59:33246Just because your CL made it through the CQ doesn't mean you're in the clear
247yet. There might be internal non-public try job failures, or bugs that went
248unnoticed during the code review process. Consider monitoring the
249[Chromium tree][chromium-tree] for about a day after your CL lands. If
250the Sheriff or anyone else brings any failures to your attention, revert the CL
251first and ask questions later. Gerrit can automatically generate revert CLs.
252
Howard Wolosky7c06f7fa2021-01-20 18:37:35253## 20. Cleanup
Toby Huang5c3c00e2019-10-30 23:29:05254
Toby Huang5105f812019-08-08 23:47:57255After your CL is landed, you can use `git rebase-update` or `git cl archive` to
Toby Huang5c3c00e2019-10-30 23:29:05256clean up your local branches. These commands will automatically delete merged
257branches. Mark the associated crbug as "fixed".
Toby Huang5105f812019-08-08 23:47:57258
259[//]: # (the reference link section should be alphabetically sorted)
John Palmer046f9872021-05-24 01:24:56260[build-instructions]: https://chromium.googlesource.com/chromium/src.git/+/main/docs/#Checking-Out-and-Building
Toby Huangfb650002020-10-07 19:59:33261[chromium-tree]: https://ci.chromium.org/p/chromium/g/main/console
Toby Huang5105f812019-08-08 23:47:57262[contributing]: contributing.md
Toby Huangd3588732021-04-01 21:47:34263[simple-chrome]: https://chromium.googlesource.com/chromiumos/docs/+/HEAD/simple_chrome_workflow.md
Toby Huang5105f812019-08-08 23:47:57264[uploading-a-change-for-review]: contributing.md#Uploading-a-change-for-review