Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 1 | # Commit Checklist for Chromium Workflow |
| 2 | |
Toby Huang | e43e5d68 | 2019-10-08 21:26:07 | [diff] [blame] | 3 | Here is a helpful checklist to go through before uploading change lists (CLs) on |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 4 | Gerrit and during the code review process. Gerrit is the code review platform |
| 5 | for the Chromium project. This checklist is designed to be streamlined. See |
Toby Huang | e43e5d68 | 2019-10-08 21:26:07 | [diff] [blame] | 6 | [contributing to Chromium][contributing] for a more thorough reference. The |
| 7 | intended audience is software engineers who are unfamiliar with contributing to |
Toby Huang | 0a0375c | 2020-02-21 08:00:28 | [diff] [blame] | 8 | the Chromium project. Feel free to skip steps that are not applicable to the |
Toby Huang | c9bd85b | 2020-07-30 22:02:35 | [diff] [blame] | 9 | patchset you're currently uploading. |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 10 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 11 | According to the Checklist Manifesto by Atul Gawande, checklists are a marvelous |
| 12 | tool for ensuring consistent quality in the work you produce. Checklists also |
| 13 | help you work more efficiently by ensuring you never skip a step or waste brain |
| 14 | power figuring out the next step to take. |
| 15 | |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 16 | [TOC] |
| 17 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 18 | ## 1. Create a new branch or switch to the correct branch |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 19 | |
| 20 | You should create a new branch before starting any development work. It's |
Toby Huang | 0a0375c | 2020-02-21 08:00:28 | [diff] [blame] | 21 | helpful 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 Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 24 | |
Toby Huang | 0a0375c | 2020-02-21 08:00:28 | [diff] [blame] | 25 | You may also want to set another local branch as the upstream branch. You can do |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 26 | that with `git checkout -b <branch_name> --track <upstream_branch>`. Do this if |
| 27 | you want to split your work across multiple CLs, but some CLs have dependencies |
| 28 | on others. |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 29 | |
Toby Huang | 5c3c00e | 2019-10-30 23:29:05 | [diff] [blame] | 30 | Mark the associated crbug as "started" so that other people know that you have |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 31 | started working on the bug. Taking this step can avoid duplicated work. |
Toby Huang | 5c3c00e | 2019-10-30 23:29:05 | [diff] [blame] | 32 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 33 | If 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 |
| 35 | few things more frustrating than to finish implementing your ideas or feedback, |
| 36 | and to spend hours debugging some mysterious bug, only to discover that the bug |
| 37 | was 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 | |
| 41 | Suppose you have a downstream branch chained to an upstream branch. If you |
| 42 | commit changes to the upstream branch, and you want the changes to appear in |
| 43 | your 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 Huang | 2c40fc5 | 2020-08-10 21:44:40 | [diff] [blame] | 51 | Expect to fix numerous merge conflicts. Use `git rebase --continue` once you're |
| 52 | done. |
| 53 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 54 | ## 3. Make your changes |
Toby Huang | ba5bbb4 | 2019-09-04 23:23:07 | [diff] [blame] | 55 | |
| 56 | Do your thing. There's no further advice here about how to write or fix code. |
| 57 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 58 | ## 4. Make sure the code builds correctly |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 59 | |
| 60 | After making your changes, check that common targets build correctly: |
| 61 | |
| 62 | * chrome (for Linux, ChromeOS, etc.) |
| 63 | * unit_tests |
| 64 | * browser_tests |
| 65 | |
Toby Huang | fb63826 | 2020-09-21 20:11:00 | [diff] [blame] | 66 | You can find [instructions here][build-instructions] for building various |
| 67 | targets. |
| 68 | |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 69 | It's easy to inadvertently break one of the other builds you're not currently |
| 70 | working on without realizing it. Even though the Commit Queue should catch any |
| 71 | build errors, checking locally first can save you some time since the CQ Dry Run |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 72 | can take a while to run, on the order of a few hours sometimes. |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 73 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 74 | ## 5. Test your changes |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 75 | |
Toby Huang | fb63826 | 2020-09-21 20:11:00 | [diff] [blame] | 76 | Test your changes manually by running the Chrome binary or deploying your |
| 77 | changes 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 |
| 79 | device. Make sure you hit every code path you changed. |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 80 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 81 | ## 6. Write unit or browser tests for any new code |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 82 | |
| 83 | Consider automating any manual testing you did in the previous step. |
| 84 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 85 | ## 7. Ensure the code is formatted nicely |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 86 | |
| 87 | Run `git cl format --js`. The `--js` option also formats JavaScript changes. |
| 88 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 89 | ## 8. Review your changes |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 90 | |
Toby Huang | 0a0375c | 2020-02-21 08:00:28 | [diff] [blame] | 91 | Use `git diff` to review all of the changes you've made from the previous |
| 92 | commit. Use `git upstream-diff` to review all of the changes you've made |
| 93 | from the upstream branch. The output from `git upstream-diff` is what will |
| 94 | be uploaded to Gerrit. |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 95 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 96 | ## 9. Stage relevant files for commit |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 97 | |
Toby Huang | 0a0375c | 2020-02-21 08:00:28 | [diff] [blame] | 98 | Run `git add <path_to_file>` for all of the files you've modified that you want |
| 99 | to include in the CL. Unlike other version-control systems such as svn, you have |
| 100 | to specifically `git add` the files you want to commit before calling |
| 101 | `git commit`. |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 102 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 103 | ## 10. Commit your changes |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 104 | |
Toby Huang | 0a0375c | 2020-02-21 08:00:28 | [diff] [blame] | 105 | Run `git commit`. Be sure to write a useful commit message. Here are some |
| 106 | [tips for writing good commit messages][uploading-a-change-for-review]. A |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 107 | shortcut for combining steps the previous step and this one is `git commit -a -m |
| 108 | <commit_message>`. |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 109 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 110 | ## 11. Squash your commits |
Toby Huang | ba5bbb4 | 2019-09-04 23:23:07 | [diff] [blame] | 111 | |
| 112 | If you have many commits on your current branch, and you want to avoid some |
Toby Huang | 0a0375c | 2020-02-21 08:00:28 | [diff] [blame] | 113 | nasty commit-by-commit merge conflicts in the next step, consider collecting all |
| 114 | your changes into one commit. Run `git rebase -i @{u}`. The `@{u}` is a |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 115 | short-hand pointer for the upstream branch, which is usually origin/master, but |
| 116 | can also be one of your local branches. After running the `git rebase` command, |
| 117 | you should see a list of commits, with each commit starting with the word |
| 118 | "pick". Make sure the first commit says "pick" and change the rest from "pick" |
| 119 | to "squash". This will squash each commit into the previous commit, which will |
| 120 | continue until each commit is squashed into the first commit. |
Toby Huang | ba5bbb4 | 2019-09-04 23:23:07 | [diff] [blame] | 121 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 122 | An alternative way to squash your commits into a single commit is to do `git |
| 123 | commit --amend` in the previous step. |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 124 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 125 | ## 12. Rebase your local repository |
| 126 | |
| 127 | Rebasing is a neat way to sync changes from the remote repository and resolve |
| 128 | any merge conflict errors on your CL. Run `git rebase-update`. This command |
| 129 | updates all of your local branches with remote changes that have landed since |
| 130 | you started development work, which could've been a while ago. It also deletes |
| 131 | any branches that match the remote repository, such as after the CL associated |
| 132 | with that branch has been merged. In summary, `git rebase-update` cleans up your |
| 133 | local branches. |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 134 | |
Toby Huang | 0a0375c | 2020-02-21 08:00:28 | [diff] [blame] | 135 | You may run into rebase conflicts. Fix them manually before proceeding with |
| 136 | `git rebase --continue`. Note that rebasing has the potential to break your |
| 137 | build, so you might want to try re-building afterwards. |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 138 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 139 | ## 13. Upload the CL to Gerrit |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 140 | |
| 141 | Run `git cl upload`. Some useful options include: |
| 142 | |
Toby Huang | c9bd85b | 2020-07-30 22:02:35 | [diff] [blame] | 143 | * `--cq-dry-run` (or `-d`) will set the patchset to do a CQ Dry Run. It is a |
| 144 | good idea to run try jobs for each new patchset with significant changes. |
Toby Huang | e43e5d68 | 2019-10-08 21:26:07 | [diff] [blame] | 145 | * `-r <chromium_username>` will add reviewers. |
| 146 | * `-b <bug_number>` automatically populates the bug reference line of the |
Toby Huang | c9bd85b | 2020-07-30 22:02:35 | [diff] [blame] | 147 | commit message. Use `-b None` is there is no relevant crbug. |
Toby Huang | fb63826 | 2020-09-21 20:11:00 | [diff] [blame] | 148 | * `--edit-description` will let you update the commit message. Using square |
| 149 | brackets in the commit message title, like [hashtag], will add a hashtag to |
| 150 | your CL. This feature is useful for grouping related CLs together. |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 151 | |
Toby Huang | c9bd85b | 2020-07-30 22:02:35 | [diff] [blame] | 152 | To help guide your reviewers, it is also recommended to provide a title for each |
| 153 | patchset summarizing the changes and indicating whose comments the patchset |
| 154 | addresses. Running `git cl upload` will upload a new patchset and prompt you for |
| 155 | a brief patchset title. The title defaults to your most recent commit summary, |
| 156 | so if you tend to squash all your commits into one, try to enter a new summary |
| 157 | each time you upload. You can also modify the patchset title directly in Gerrit. |
| 158 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 159 | ## 14. Check the CL again in Gerrit |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 160 | |
| 161 | Run `git cl web` to go to the Gerrit URL associated with the current branch. |
Toby Huang | c9bd85b | 2020-07-30 22:02:35 | [diff] [blame] | 162 | Open the latest patchset and verify that all of the uploaded files are correct. |
Toby Huang | e43e5d68 | 2019-10-08 21:26:07 | [diff] [blame] | 163 | Click `Expand All` to check over all of the individual line-by-line changes |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 164 | again. Basically do a self-review before asking your reviewers for a review. |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 165 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 166 | ## 15. Make sure all auto-regression tests pass |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 167 | |
| 168 | Click `CQ Dry Run`. Fix any errors because otherwise the CL won't pass the |
| 169 | commit queue (CQ) checks. Consider waiting for the CQ Dry Run to pass before |
| 170 | notifying your reviewers, in case the results require major changes in your CL. |
| 171 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 172 | ## 16. Add reviewers to review your code |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 173 | |
| 174 | Click `Find Owners` or run `git cl owners` to find file owners to review your |
| 175 | code and instruct them about which parts you want them to focus on. Add anyone |
Toby Huang | e43e5d68 | 2019-10-08 21:26:07 | [diff] [blame] | 176 | else you think should review your code. The blame functionality in Code Search |
| 177 | is a good way to identify reviewers who may be familiar with the parts of code |
| 178 | your CL touches. For your CL to land, you need an approval from an owner for |
| 179 | each file you've changed, unless you are an owner of some files, in which case |
| 180 | you don't need separate owner approval for those files. |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 181 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 182 | ## 17. Implement feedback from your reviewers |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 183 | |
| 184 | Then go through this commit checklist again. Reply to all comments from the |
| 185 | reviewers on Gerrit and mark all resolved issues as resolved (clicking `Done` or |
| 186 | `Ack` will do this automatically). Click `Reply` to ensure that your reviewers |
| 187 | receive a notification. Doing this signals that your CL is ready for review |
| 188 | again, since the assumption is that your CL is not ready for review until you |
| 189 | hit reply. |
| 190 | |
Toby Huang | fb63826 | 2020-09-21 20:11:00 | [diff] [blame] | 191 | If your change is simple and you feel confident that your reviewer will approve |
| 192 | your CL on the next iteration, you can set Auto-Submit +1. The CL will proceed |
| 193 | to the next step automatically after approval. This feature is useful if your |
| 194 | reviewer is in a different time zone and you want to land the CL sooner. Setting |
| 195 | this flag also puts the onus on your reviewer to land the CL. |
| 196 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 197 | ## 18. Land your CL |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 198 | |
| 199 | Once you have obtained a Looks Good To Me (LGTM), which is reflected by a |
| 200 | Code-Review+1 in Gerrit, from at least one owner for each file, then you have |
| 201 | the minimum prerequisite to land your changes. It may be helpful to wait for all |
| 202 | of your reviewers to approve your changes as well, even if they're not owners. |
Toby Huang | fb63826 | 2020-09-21 20:11:00 | [diff] [blame] | 203 | Don't use `chrome/OWNERS` as a blanket stamp if your CL makes significant |
| 204 | changes to subsystems. Click `Submit to CQ` to try your change in the commit |
| 205 | queue (CQ), which will land it if successful. |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 206 | |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 207 | ## 19. Cleanup |
Toby Huang | 5c3c00e | 2019-10-30 23:29:05 | [diff] [blame] | 208 | |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 209 | After your CL is landed, you can use `git rebase-update` or `git cl archive` to |
Toby Huang | 5c3c00e | 2019-10-30 23:29:05 | [diff] [blame] | 210 | clean up your local branches. These commands will automatically delete merged |
| 211 | branches. Mark the associated crbug as "fixed". |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 212 | |
| 213 | [//]: # (the reference link section should be alphabetically sorted) |
Toby Huang | fb63826 | 2020-09-21 20:11:00 | [diff] [blame] | 214 | [build-instructions]: https://chromium.googlesource.com/chromium/src.git/+/master/docs/#Checking-Out-and-Building |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 215 | [contributing]: contributing.md |
Toby Huang | acb9beba | 2020-06-25 20:08:04 | [diff] [blame] | 216 | [simple-chrome]: https://chromium.googlesource.com/chromiumos/docs/+/master/simple_chrome_workflow.md |
Toby Huang | 5105f81 | 2019-08-08 23:47:57 | [diff] [blame] | 217 | [uploading-a-change-for-review]: contributing.md#Uploading-a-change-for-review |