blob: cf5924bba383bc29bb7b62ae329ba5776e7badfd [file] [log] [blame] [view]
Daniel Cheng86135f32019-02-27 07:10:381# Contributing to Chromium
2
3This page assumes a working Chromium [checkout and build][checkout-and-build].
4Note that a full Chromium checkout includes external repositories with their
5own workflows for contributing, such as [v8][v8-dev-guide] and
6[Skia][skia-dev-guide]. Similarly, ChromiumOS, which includes Chromium as a
7subrepository, has its own [development workflow][cros-dev-guide].
8
9[TOC]
10
11## Related resources
12
13- [Life of a Chromium Developer][life-of-a-chromium-developer], which is mostly
14 up-to-date.
15- [Tutorial][noms-tutorial] by committer emeritus [email protected]
16
17## Communicate
18
19When writing a new feature or fixing an existing bug, get a second opinion
20before going too far. If it's a new feature idea, propose it to the appropriate
21[discussion group][discussion-groups]. If it's in the existing code base, talk
22to some of the folks in the "OWNERS" file (see [code review
23policies][code-reviews] for more) for the code being changed.
24
25- If a change needs further context outside the CL, it should be tracked in the
26 [bug system][crbug]. Bugs are the right place for long histories, discussion
27 and debate, attaching screenshots, and linking to other associated bugs. Bugs
28 are unnecessary for changes isolated enough to need none of these.
29- If there isn't a bug and there should be one, please [file a new
30 bug][crbug-new].
31- Just because there is a bug in the bug system doesn't necessarily mean that a
32 patch will be accepted.
33
34## Legal stuff
35
36All contributors must complete the contributor license agreement. For
37individual contributors, please complete the [Individual Contributor License
38Agreement][individual-cla] online. Corporate contributors must fill out the
39[Corporate Contributor License Agreement][corporate-cla] and send it to us as
40described on that page.
41
42### First-time contributors
43
44Add your (or your organization's) name and contact info to the AUTHORS file for
45[Chromium][cr-authors] or [Chromium OS][cros-authors]. Please include this as
46part of your first patch and not as a separate standalone patch.
47
48### External contributor checklist for reviewers
49
50Before LGTMing a change from a non-chromium.org address, ensure that the
51contribution can be accepted:
52
53- Definition: The "author" is the email address that owns the code review
54 request on <https://chromium-review.googlesource.com>
55- Ensure the author is already listed in [AUTHORS][cr-authors]. In some cases, the
56 author's company might have a wildcard rule (e.g. \*@google.com).
57- If the author or their company is not listed, the CL should include a new
58 AUTHORS entry.
59 - Ensure the new entry is reviewed by a reviewer who works for Google.
60 - If there is a corporate CLA for the author's company, it must list the
61 person explicitly (or the list of authorized contributors must say
62 something like "All employees"). If the author is not on their company's
63 roster, do not accept the change.
64
65## Initial git setup
66
671. Visit <https://chromium-review.googlesource.com/new-password> and follow the
68 on-screen instructions to get credentials for uploading changes.
692. Tell git about your name, email and some other settings.
70 ```
71 git config --global user.name "My Name"
72 git config --global user.email "[email protected]"
73 git config --global core.autocrlf false
74 git config --global core.filemode false
75 git config --local gerrit.host true
76 # Uncomment this if you want your pull commands to always rebase.
77 # git config --global branch.autosetuprebase always
78 # Uncomment if you want new branches to track the current branch.
79 # git config --global branch.autosetupmerge always
80 ```
81
82## Creating a change
83
84First, create a new branch for your change in git. Here, we create a branch
85called `mychange` (use whatever name you want here), with `origin/master` as
86the upstream branch.
87
88```
89git checkout -b mychange -t origin/master
90```
91
92Write and test your change.
93
94- Conform to the [style guide][cr-styleguide].
95- Include tests.
96- Patches should be a reasonable size to review. Review time often increases
97 expontentially with patch size.
98
99Commit your change locally in git:
100
101```
102git commit -a
103```
104
105If you are not familiar with `git`, GitHub's [resources to learn
106git][github-tutorial] is useful for the basics. However, keep in mind that the
107Chromium workflow is not the same as the GitHub pull request workflow.
108
109## Uploading a change for review
110
111Chromium uses a Gerrit instance hosted at
112<https://chromium-review.googlesource.com> for code reviews. In order to upload
113your local change to Gerrit, use `git-cl` from
114[depot\_tools][depot-tools-setup] to create a new Gerrit change, based on the
115diff between the current branch and its upstream branch:
116
117```
118git cl upload
119```
120
121This will open a text editor to create a description for the new change. This
122description will be used as the commit message when the change is landed in the
123Chromium tree. Descriptions should be formatted as follows:
124
125```
126Summary of change (one line)
127
128Longer description of change addressing as appropriate: why the change
129is made, context if it is part of many changes, description of previous
130behavior and newly introduced differences, etc.
131
132Long lines should be wrapped to 72 columns for easier log message
133viewing in terminals.
134
135Bug: 123456
136```
137
138A short subject and a blank line after the subject are crucial: `git` uses this
139as a heuristic for tools like `git log --oneline`. Use the bug number from the
140[issue tracker][crbug] (see more on [CL footer syntax][cl-footer-syntax]). Also
141see [How to Write a Git Commit Message][good-git-commit-message], which has more
142in-depth tips for writing a good commit description.
143
144### Chromium-specific description tips
145
146- Links to previous CLs should be formatted as `https://crrev.com/c/NUMBER`,
147 which forwards to [Gitiles][cr-gitiles], rather than linking to the review at
148 <https://chromium-review.googlesource.com>.
149
150- If there are instructions for testers to verify the change is correct,
151 include them with the `Test:` tag:
152
153 ```
154 Test: Load example.com/page.html and click the foo-button; see
155 crbug.com/123456 for more details.
156 ```
157
158After saving the change description, `git-cl` runs some presubmit scripts to
159check for common errors. If everything passes, `git-cl` will print something
160like this:
161
162```
163remote: SUCCESS
164remote:
165remote: New Changes:
166remote: https://chromium-review.googlesource.com/c/chromium/src/+/1485699 Use base::TimeDelta::FromTimeSpec helper in more places. [WIP]
167```
168
169Additional flags can be used to specify reviewers, bugs fixed by the change, et
170cetera:
171
172```
173git cl upload -r [email protected],[email protected] -b 123456
174```
175
176See `git cl help upload` for a full list of flags.
177
178## Code review
179
180Code reviews are covered in more detail on the [code review
181policies][code-reviews] page.
182
183### Finding a reviewer
184
185Ideally, the reviewer is someone who is familiar with the area of code in
186question. If you're not sure who that should be, check with anyone in the
187nearest ancestor OWNERS file.
188
189- Anybody can review code, but there must be at least one owner for each
190 affected directory.
191- If there are multiple reviewers, make it clear what each reviewer is expected
192 to review. Otherwise, people might assume their input is not required or
193 waste time with redundant reviews.
194- `git cl owners` automatically suggests reviewers based on the OWNERS files.
195
196### Requesting review
197
198Open the change on [the web][crrev]. If you can't find the link, running `git
199cl issue` will display the review URL for the current branch. Alternatively,
200visit <https://chromium-review.googlesource.com> and look in the "Outgoing
201Reviews" section.
202
203Reviewers expect to review code that compiles and passes tests. If you have
204access, now is a good time to run your change through the [automated
205tests](#running-automated-tests).
206
207Click **Add Reviewers** in the left column (if you don't see this link, make
208sure you are logged in). In the **Reviewers** field, enter a comma-separated
209list of the reviewers you picked.
210
211In the same dialog, you can include an optional message to your reviewers. This
212space can be used for specific questions or instructions. Once you're done,
213make sure to click **Send**, which notifies the requested reviewers that they
214should review your change.
215
216**IMPORTANT: UNTIL YOU SEND THE REVIEW REQUEST, NO ONE WILL LOOK AT THE REVIEW**
217
218### Review process
219
220All changes must be reviewed (see [code review policies][code-reviews]).
221
222You should get a response within **one** business day; re-ping your reviewers
223if you do not.
224
225To upload new patch sets that address comments from the reviewers, simply
226commit more changes to your local branch and run `git cl upload` again.
227
228### Approval
229
230When the reviewer is happy with the change, they will set the "Code-Review +1"
231label. Owners of all affected files must approve before a change can be
232committed. See: [code review policies: owners][code-reviews-owners].
233
234## Running automated tests
235
236Before being submitted, a change must pass the commit queue (CQ). The commit
237queue is an automated system which sends a patch to multiple try bots running
238different platforms: each try bot compiles Chromium with the patch and ensures
239the tests still pass on that platform.
240
241To trigger this process, click **CQ Dry Run** in the upper right corner of the
242code review tool. Note that this is equivalent to setting the "Commit-Queue +1"
243label. Anyone can set this label; however, the CQ will not process the patch
244unless the person setting the label has [try job access][try-job-access].
245
246If you don't have try job access and:
247
248- you have an @chromium.org email address, request access for yourself.
249- you have contributed a few patches, ask a reviewer to nominate you for access.
250- neither of the above is true, request that a reviewer run try jobs for you in
251 the code review request message.
252
253The status of the latest try job for a given patchset is visible just below the
254list of changed files. Each bot has its own bubble, using one of the following
255colors to indicate its status:
256
257- Gray: the bot has not started processing the patch yet.
258- Yellow: the run is in progress. Check back later!
259- Purple: the trybot encountered an exception while processing the patch.
260 Usually, this is not the fault of the patch. Try clicking **CQ Dry Run**
261 again.
262- Red: tests failed. Click on the failed bot to see what tests failed and why.
263- Green: the run passed!
264
265## Committing
266
267Changes should generally be committed via the [commit queue][commit-queue].
268This is done by clicking **Submit to CQ** in the upper right corner, or setting
269the "Commit-Queue +2" label on the change. The commit queue will then
270send the patch to the try bots. If all try bots return green, the change will
271automatically be committed. Yay!
272
273Sometimes a test might be flaky. If you have an isolated failure that appears
274unrelated to your change, try sending the change to the commit queue again.
275
276Alternatively, a developer with commit access can [directly
277commit][direct-commit] a change, bypassing the commit queue. This should only
278be used in emergencies because it will bypass all the safety nets.
279
280## Tips
281
Dominik Röttschesd113bfa2019-07-10 08:56:24282### Review etiquette
283
Daniel Cheng86135f32019-02-27 07:10:38284During the lifetime of a review, you may want to rebase your change onto a newer
285source revision to minimize merge conflicts. The reviewer-friendly way to do
286this is to first address any unresolved comments and upload those changes as a
287patchset. Then, rebase to the newer revision and upload that as its own
288patchset (with no other changes). This makes it easy for reviewers to see the
289changes made in response to their comments, and then quickly verify the diffs
290from the rebase.
291
292Code authors and reviewers should keep in mind that Chromium is a global
293project: contributors and reviewers are often in time zones far apart. Please
294read these guidelines on [minimizing review lag][review-lag] and take them in
295consideration both when writing reviews and responding to review feedback.
296
Dominik Röttschesd113bfa2019-07-10 08:56:24297### Watchlists
298
299If you would like to be notified about changes to a set of files covering a
300topic or an area of Chromium, you may use the [watchlists][watchlist-doc]
301feature in order to receive email notifications.
302
303
Daniel Cheng86135f32019-02-27 07:10:38304[//]: # (the reference link section should be alphabetically sorted)
305[checkout-and-build]: https://chromium.googlesource.com/chromium/src/+/master/docs/#checking-out-and-building
306[cl-footer-syntax]: https://dev.chromium.org/developers/contributing-code/-bug-syntax
307[code-reviews-owners]: code_reviews.md#OWNERS-files
308[code-reviews]: code_reviews.md
309[commit-queue]: infra/cq.md
310[core-principles]: https://www.chromium.org/developers/core-principles
311[corporate-cla]: https://cla.developers.google.com/about/google-corporate?csw=1
312[cr-authors]: https://chromium.googlesource.com/chromium/src/+/HEAD/AUTHORS
313[cr-gitiles]: https://chromium.googlesource.com/chromium/src/+/master/
314[cr-styleguide]: https://chromium.googlesource.com/chromium/src/+/master/styleguide/styleguide.md
315[crbug-new]: https://bugs.chromium.org/p/chromium/issues/entry
316[crbug]: https://bugs.chromium.org/p/chromium/issues/list
317[cros-authors]: https://chromium.googlesource.com/chromium/src/+/master/AUTHORS
318[cros-dev-guide]: https://chromium.googlesource.com/chromiumos/docs/+/master/developer_guide.md
319[crrev]: https://chromium-review.googlesource.com
320[depot-tools-setup]: https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/depot_tools_tutorial.html#_setting_up
321[direct-commit]: https://dev.chromium.org/developers/contributing-code/direct-commit
322[discussion-groups]: https://www.chromium.org/developers/discussion-groups
323[github-tutorial]: https://try.github.io
324[good-git-commit-message]: https://chris.beams.io/posts/git-commit/
325[individual-cla]: https://cla.developers.google.com/about/google-individual?csw=1
326[life-of-a-chromium-developer]: https://docs.google.com/a/google.com/present/view?id=0AetfwCoL2lQAZGQ5bXJ0NDVfMGRtdGQ0OWM2
327[noms-tutorial]: https://meowni.ca/posts/chromium-101
328[review-lag]: https://dev.chromium.org/developers/contributing-code/minimizing-review-lag-across-time-zones
329[skia-dev-guide]: https://skia.org/dev/contrib
330[try-job-access]: https://www.chromium.org/getting-involved/become-a-committer#TOC-Try-job-access
331[v8-dev-guide]: https://v8.dev/docs
Dominik Röttschesd113bfa2019-07-10 08:56:24332[watchlist-doc]: infra/watchlists.md