blob: fc801410d91e9a81ea1875ffe0b7b3b9d91a5cc4 [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
23`git checkout -b <branch_name> --track origin/master`.
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
28on others.
Toby Huang5105f812019-08-08 23:47:5729
Toby Huang5c3c00e2019-10-30 23:29:0530Mark the associated crbug as "started" so that other people know that you have
Toby Huangacb9beba2020-06-25 20:08:0431started working on the bug. Taking this step can avoid duplicated work.
Toby Huang5c3c00e2019-10-30 23:29:0532
Toby Huangacb9beba2020-06-25 20:08:0433If you have already created a branch, don't forget to `git checkout
34<branch_name>` to the correct branch before resuming development work. There's
35few things more frustrating than to finish implementing your ideas or feedback,
36and to spend hours debugging some mysterious bug, only to discover that the bug
37was caused by working on the wrong branch this whole time.
38
39## 2. If there's a local upstream branch, rebase the upstream changes
40
41Suppose you have a downstream branch chained to an upstream branch. If you
42commit changes to the upstream branch, and you want the changes to appear in
43your downstream branch, you need to:
44
45* `git checkout <branch_name>` to the downstream branch.
46* Run `git rebase -i @{u}` to pull the upstream changes into the current
47 branch.
48* Run `git rebase -i @{u}` again to rebase the downstream changes onto the
49 upstream branch.
50
Toby Huang2c40fc52020-08-10 21:44:4051Expect to fix numerous merge conflicts. Use `git rebase --continue` once you're
52done.
53
Toby Huangacb9beba2020-06-25 20:08:0454## 3. Make your changes
Toby Huangba5bbb42019-09-04 23:23:0755
56Do your thing. There's no further advice here about how to write or fix code.
57
Toby Huangacb9beba2020-06-25 20:08:0458## 4. Make sure the code builds correctly
Toby Huang5105f812019-08-08 23:47:5759
60After making your changes, check that common targets build correctly:
61
62* chrome (for Linux, ChromeOS, etc.)
63* unit_tests
64* browser_tests
65
Toby Huangfb638262020-09-21 20:11:0066You can find [instructions here][build-instructions] for building various
67targets.
68
Toby Huang5105f812019-08-08 23:47:5769It's easy to inadvertently break one of the other builds you're not currently
70working on without realizing it. Even though the Commit Queue should catch any
71build errors, checking locally first can save you some time since the CQ Dry Run
Toby Huangacb9beba2020-06-25 20:08:0472can take a while to run, on the order of a few hours sometimes.
Toby Huang5105f812019-08-08 23:47:5773
Toby Huangacb9beba2020-06-25 20:08:0474## 5. Test your changes
Toby Huang5105f812019-08-08 23:47:5775
Toby Huangfb638262020-09-21 20:11:0076Test your changes manually by running the Chrome binary or deploying your
77changes to a test device. If you're testing Chrome for ChromeOS, follow the
78[Simple Chrome][simple-chrome] instructions to deploy your changes to a test
79device. Make sure you hit every code path you changed.
Toby Huang5105f812019-08-08 23:47:5780
Toby Huangb54e13322020-10-29 18:38:0281Think about testing any edge cases that could break your code. Some common edge
82cases to consider:
83
84* Guest mode
85* Enterprise/EDU/Supervised users
86* Accessibility
87* Official Chrome-branded build (for Googlers)
88
Toby Huangacb9beba2020-06-25 20:08:0489## 6. Write unit or browser tests for any new code
Toby Huang5105f812019-08-08 23:47:5790
91Consider automating any manual testing you did in the previous step.
92
Toby Huangacb9beba2020-06-25 20:08:0493## 7. Ensure the code is formatted nicely
Toby Huang5105f812019-08-08 23:47:5794
95Run `git cl format --js`. The `--js` option also formats JavaScript changes.
96
Toby Huangacb9beba2020-06-25 20:08:0497## 8. Review your changes
Toby Huang5105f812019-08-08 23:47:5798
Toby Huang0a0375c2020-02-21 08:00:2899Use `git diff` to review all of the changes you've made from the previous
100commit. Use `git upstream-diff` to review all of the changes you've made
101from the upstream branch. The output from `git upstream-diff` is what will
102be uploaded to Gerrit.
Toby Huang5105f812019-08-08 23:47:57103
Toby Huangacb9beba2020-06-25 20:08:04104## 9. Stage relevant files for commit
Toby Huang5105f812019-08-08 23:47:57105
Toby Huang0a0375c2020-02-21 08:00:28106Run `git add <path_to_file>` for all of the files you've modified that you want
107to include in the CL. Unlike other version-control systems such as svn, you have
108to specifically `git add` the files you want to commit before calling
109`git commit`.
Toby Huang5105f812019-08-08 23:47:57110
Toby Huangacb9beba2020-06-25 20:08:04111## 10. Commit your changes
Toby Huang5105f812019-08-08 23:47:57112
Toby Huang0a0375c2020-02-21 08:00:28113Run `git commit`. Be sure to write a useful commit message. Here are some
114[tips for writing good commit messages][uploading-a-change-for-review]. A
Toby Huangfb650002020-10-07 19:59:33115shortcut for combining the previous step and this one is `git commit -a -m
Toby Huangacb9beba2020-06-25 20:08:04116<commit_message>`.
Toby Huang5105f812019-08-08 23:47:57117
Toby Huangacb9beba2020-06-25 20:08:04118## 11. Squash your commits
Toby Huangba5bbb42019-09-04 23:23:07119
120If you have many commits on your current branch, and you want to avoid some
Toby Huang0a0375c2020-02-21 08:00:28121nasty commit-by-commit merge conflicts in the next step, consider collecting all
122your changes into one commit. Run `git rebase -i @{u}`. The `@{u}` is a
Toby Huangacb9beba2020-06-25 20:08:04123short-hand pointer for the upstream branch, which is usually origin/master, but
124can also be one of your local branches. After running the `git rebase` command,
125you should see a list of commits, with each commit starting with the word
126"pick". Make sure the first commit says "pick" and change the rest from "pick"
127to "squash". This will squash each commit into the previous commit, which will
128continue until each commit is squashed into the first commit.
Toby Huangba5bbb42019-09-04 23:23:07129
Toby Huangacb9beba2020-06-25 20:08:04130An alternative way to squash your commits into a single commit is to do `git
131commit --amend` in the previous step.
Toby Huang5105f812019-08-08 23:47:57132
Toby Huangacb9beba2020-06-25 20:08:04133## 12. Rebase your local repository
134
135Rebasing is a neat way to sync changes from the remote repository and resolve
136any merge conflict errors on your CL. Run `git rebase-update`. This command
137updates all of your local branches with remote changes that have landed since
138you started development work, which could've been a while ago. It also deletes
139any branches that match the remote repository, such as after the CL associated
140with that branch has been merged. In summary, `git rebase-update` cleans up your
141local branches.
Toby Huang5105f812019-08-08 23:47:57142
Toby Huang0a0375c2020-02-21 08:00:28143You may run into rebase conflicts. Fix them manually before proceeding with
Toby Huangfb650002020-10-07 19:59:33144`git rebase --continue`.
145
146Note that rebasing has the potential to break your build, so you might want to
147try re-building afterwards. You need to run `gclient sync -D` before trying to
148build again after a rebase-update, to update third-party dependencies.
Toby Huang5105f812019-08-08 23:47:57149
Toby Huangacb9beba2020-06-25 20:08:04150## 13. Upload the CL to Gerrit
Toby Huang5105f812019-08-08 23:47:57151
152Run `git cl upload`. Some useful options include:
153
Toby Huangc9bd85b2020-07-30 22:02:35154* `--cq-dry-run` (or `-d`) will set the patchset to do a CQ Dry Run. It is a
155 good idea to run try jobs for each new patchset with significant changes.
Toby Huange43e5d682019-10-08 21:26:07156* `-r <chromium_username>` will add reviewers.
157* `-b <bug_number>` automatically populates the bug reference line of the
Toby Huangc9bd85b2020-07-30 22:02:35158 commit message. Use `-b None` is there is no relevant crbug.
Toby Huangfb638262020-09-21 20:11:00159* `--edit-description` will let you update the commit message. Using square
160 brackets in the commit message title, like [hashtag], will add a hashtag to
161 your CL. This feature is useful for grouping related CLs together.
Toby Huang5105f812019-08-08 23:47:57162
Toby Huangc9bd85b2020-07-30 22:02:35163To help guide your reviewers, it is also recommended to provide a title for each
164patchset summarizing the changes and indicating whose comments the patchset
165addresses. Running `git cl upload` will upload a new patchset and prompt you for
166a brief patchset title. The title defaults to your most recent commit summary,
167so if you tend to squash all your commits into one, try to enter a new summary
168each time you upload. You can also modify the patchset title directly in Gerrit.
169
Toby Huangacb9beba2020-06-25 20:08:04170## 14. Check the CL again in Gerrit
Toby Huang5105f812019-08-08 23:47:57171
172Run `git cl web` to go to the Gerrit URL associated with the current branch.
Toby Huangc9bd85b2020-07-30 22:02:35173Open the latest patchset and verify that all of the uploaded files are correct.
Toby Huange43e5d682019-10-08 21:26:07174Click `Expand All` to check over all of the individual line-by-line changes
Toby Huangacb9beba2020-06-25 20:08:04175again. Basically do a self-review before asking your reviewers for a review.
Toby Huang5105f812019-08-08 23:47:57176
Toby Huangacb9beba2020-06-25 20:08:04177## 15. Make sure all auto-regression tests pass
Toby Huang5105f812019-08-08 23:47:57178
179Click `CQ Dry Run`. Fix any errors because otherwise the CL won't pass the
180commit queue (CQ) checks. Consider waiting for the CQ Dry Run to pass before
181notifying your reviewers, in case the results require major changes in your CL.
182
Toby Huangacb9beba2020-06-25 20:08:04183## 16. Add reviewers to review your code
Toby Huang5105f812019-08-08 23:47:57184
185Click `Find Owners` or run `git cl owners` to find file owners to review your
186code and instruct them about which parts you want them to focus on. Add anyone
Toby Huange43e5d682019-10-08 21:26:07187else you think should review your code. The blame functionality in Code Search
188is a good way to identify reviewers who may be familiar with the parts of code
189your CL touches. For your CL to land, you need an approval from an owner for
190each file you've changed, unless you are an owner of some files, in which case
191you don't need separate owner approval for those files.
Toby Huang5105f812019-08-08 23:47:57192
Howard Wolosky7c06f7fa2021-01-20 18:37:35193## 17. Start Your Review
194
195Click on the `Start Review` button to begin the actual review process. Until
196you press this button, nobody will look at your change. Once pressed, you'll
197have the opportunity to include an additional message in the notification sent
198to your reviewers.
199
200## 18. Implement feedback from your reviewers
Toby Huang5105f812019-08-08 23:47:57201
202Then go through this commit checklist again. Reply to all comments from the
203reviewers on Gerrit and mark all resolved issues as resolved (clicking `Done` or
204`Ack` will do this automatically). Click `Reply` to ensure that your reviewers
205receive a notification. Doing this signals that your CL is ready for review
206again, since the assumption is that your CL is not ready for review until you
207hit reply.
208
Toby Huangfb638262020-09-21 20:11:00209If your change is simple and you feel confident that your reviewer will approve
210your CL on the next iteration, you can set Auto-Submit +1. The CL will proceed
211to the next step automatically after approval. This feature is useful if your
212reviewer is in a different time zone and you want to land the CL sooner. Setting
213this flag also puts the onus on your reviewer to land the CL.
214
Howard Wolosky7c06f7fa2021-01-20 18:37:35215## 19. Land your CL
Toby Huang5105f812019-08-08 23:47:57216
217Once you have obtained a Looks Good To Me (LGTM), which is reflected by a
218Code-Review+1 in Gerrit, from at least one owner for each file, then you have
219the minimum prerequisite to land your changes. It may be helpful to wait for all
220of your reviewers to approve your changes as well, even if they're not owners.
Toby Huangfb638262020-09-21 20:11:00221Don't use `chrome/OWNERS` as a blanket stamp if your CL makes significant
222changes to subsystems. Click `Submit to CQ` to try your change in the commit
223queue (CQ), which will land it if successful.
Toby Huang5105f812019-08-08 23:47:57224
Toby Huangfb650002020-10-07 19:59:33225Just because your CL made it through the CQ doesn't mean you're in the clear
226yet. There might be internal non-public try job failures, or bugs that went
227unnoticed during the code review process. Consider monitoring the
228[Chromium tree][chromium-tree] for about a day after your CL lands. If
229the Sheriff or anyone else brings any failures to your attention, revert the CL
230first and ask questions later. Gerrit can automatically generate revert CLs.
231
Howard Wolosky7c06f7fa2021-01-20 18:37:35232## 20. Cleanup
Toby Huang5c3c00e2019-10-30 23:29:05233
Toby Huang5105f812019-08-08 23:47:57234After your CL is landed, you can use `git rebase-update` or `git cl archive` to
Toby Huang5c3c00e2019-10-30 23:29:05235clean up your local branches. These commands will automatically delete merged
236branches. Mark the associated crbug as "fixed".
Toby Huang5105f812019-08-08 23:47:57237
238[//]: # (the reference link section should be alphabetically sorted)
Toby Huangfb638262020-09-21 20:11:00239[build-instructions]: https://chromium.googlesource.com/chromium/src.git/+/master/docs/#Checking-Out-and-Building
Toby Huangfb650002020-10-07 19:59:33240[chromium-tree]: https://ci.chromium.org/p/chromium/g/main/console
Toby Huang5105f812019-08-08 23:47:57241[contributing]: contributing.md
Toby Huangacb9beba2020-06-25 20:08:04242[simple-chrome]: https://chromium.googlesource.com/chromiumos/docs/+/master/simple_chrome_workflow.md
Toby Huang5105f812019-08-08 23:47:57243[uploading-a-change-for-review]: contributing.md#Uploading-a-change-for-review