blob: 3284f834ac5c4a16061f7362c9f2e93371caee3b [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.
Toby Huang5105f812019-08-08 23:47:5715- [Tutorial][noms-tutorial] by committer emeritus [email protected].
16- [Commit Checklist][commit-checklist], a useful checklist to go through before
17 submitting each CL on Gerrit.
Daniel Cheng86135f32019-02-27 07:10:3818
19## Communicate
20
21When writing a new feature or fixing an existing bug, get a second opinion
22before going too far. If it's a new feature idea, propose it to the appropriate
23[discussion group][discussion-groups]. If it's in the existing code base, talk
24to some of the folks in the "OWNERS" file (see [code review
25policies][code-reviews] for more) for the code being changed.
26
27- If a change needs further context outside the CL, it should be tracked in the
28 [bug system][crbug]. Bugs are the right place for long histories, discussion
29 and debate, attaching screenshots, and linking to other associated bugs. Bugs
30 are unnecessary for changes isolated enough to need none of these.
31- If there isn't a bug and there should be one, please [file a new
32 bug][crbug-new].
33- Just because there is a bug in the bug system doesn't necessarily mean that a
34 patch will be accepted.
35
John Abd-El-Malek27e1cf02019-12-18 17:35:1836## Design Documents
37Any nontrivial technical effort that will significantly impact Chromium should
38have a design doc ([template][design-doc-template]). Specifically, we require
39design docs in the following cases:
40- When writing code that will have a large impact on Chromium as a whole, e.g.
41 when you are changing code in Chromium's critical path (page loading,
42 rendering).
43- When beginning a large technical undertaking that should be documented for
44 historical reasons (>1 person-month of work can be used as a general guideline).
45
46Send public design docs to
47[[email protected]][chromium-design-docs]. Google internal Chrome
48design docs should follow the process at
49[go/chrome-dd-review-process][chrome-dd-review-process].
50
Daniel Cheng86135f32019-02-27 07:10:3851## Legal stuff
52
53All contributors must complete the contributor license agreement. For
54individual contributors, please complete the [Individual Contributor License
55Agreement][individual-cla] online. Corporate contributors must fill out the
56[Corporate Contributor License Agreement][corporate-cla] and send it to us as
57described on that page.
58
59### First-time contributors
60
61Add your (or your organization's) name and contact info to the AUTHORS file for
62[Chromium][cr-authors] or [Chromium OS][cros-authors]. Please include this as
63part of your first patch and not as a separate standalone patch.
64
65### External contributor checklist for reviewers
66
67Before LGTMing a change from a non-chromium.org address, ensure that the
68contribution can be accepted:
69
70- Definition: The "author" is the email address that owns the code review
71 request on <https://chromium-review.googlesource.com>
72- Ensure the author is already listed in [AUTHORS][cr-authors]. In some cases, the
73 author's company might have a wildcard rule (e.g. \*@google.com).
74- If the author or their company is not listed, the CL should include a new
75 AUTHORS entry.
76 - Ensure the new entry is reviewed by a reviewer who works for Google.
Vincent Scheib04582d842020-04-24 18:41:3677 - Contributor License Agreement can be verified by Googlers at http://go/cla.
Daniel Cheng86135f32019-02-27 07:10:3878 - If there is a corporate CLA for the author's company, it must list the
79 person explicitly (or the list of authorized contributors must say
80 something like "All employees"). If the author is not on their company's
81 roster, do not accept the change.
82
83## Initial git setup
84
851. Visit <https://chromium-review.googlesource.com/new-password> and follow the
86 on-screen instructions to get credentials for uploading changes.
872. Tell git about your name, email and some other settings.
88 ```
89 git config --global user.name "My Name"
90 git config --global user.email "[email protected]"
91 git config --global core.autocrlf false
92 git config --global core.filemode false
93 git config --local gerrit.host true
94 # Uncomment this if you want your pull commands to always rebase.
95 # git config --global branch.autosetuprebase always
96 # Uncomment if you want new branches to track the current branch.
97 # git config --global branch.autosetupmerge always
98 ```
Francois Marier197916f2020-01-16 02:23:02993. Visit <https://chromium-review.googlesource.com/settings/> to ensure that
100 your preferred email is set to the same one you use in your git
101 configuration.
Daniel Cheng86135f32019-02-27 07:10:38102
103## Creating a change
104
105First, create a new branch for your change in git. Here, we create a branch
106called `mychange` (use whatever name you want here), with `origin/master` as
107the upstream branch.
108
109```
110git checkout -b mychange -t origin/master
111```
112
113Write and test your change.
114
115- Conform to the [style guide][cr-styleguide].
116- Include tests.
117- Patches should be a reasonable size to review. Review time often increases
Joshua Berenhaus98d2fbc2020-01-07 18:50:42118 exponentially with patch size.
Daniel Cheng86135f32019-02-27 07:10:38119
120Commit your change locally in git:
121
122```
123git commit -a
124```
125
126If you are not familiar with `git`, GitHub's [resources to learn
127git][github-tutorial] is useful for the basics. However, keep in mind that the
128Chromium workflow is not the same as the GitHub pull request workflow.
129
130## Uploading a change for review
131
Toby Huang5105f812019-08-08 23:47:57132Note: go through the [commit checklist][commit-checklist] for Chromium before
133uploading a change for review.
134
Daniel Cheng86135f32019-02-27 07:10:38135Chromium uses a Gerrit instance hosted at
136<https://chromium-review.googlesource.com> for code reviews. In order to upload
137your local change to Gerrit, use `git-cl` from
138[depot\_tools][depot-tools-setup] to create a new Gerrit change, based on the
139diff between the current branch and its upstream branch:
140
141```
142git cl upload
143```
144
145This will open a text editor to create a description for the new change. This
146description will be used as the commit message when the change is landed in the
147Chromium tree. Descriptions should be formatted as follows:
148
149```
150Summary of change (one line)
151
152Longer description of change addressing as appropriate: why the change
153is made, context if it is part of many changes, description of previous
154behavior and newly introduced differences, etc.
155
156Long lines should be wrapped to 72 columns for easier log message
157viewing in terminals.
158
159Bug: 123456
160```
161
162A short subject and a blank line after the subject are crucial: `git` uses this
163as a heuristic for tools like `git log --oneline`. Use the bug number from the
164[issue tracker][crbug] (see more on [CL footer syntax][cl-footer-syntax]). Also
165see [How to Write a Git Commit Message][good-git-commit-message], which has more
166in-depth tips for writing a good commit description.
167
168### Chromium-specific description tips
169
170- Links to previous CLs should be formatted as `https://crrev.com/c/NUMBER`,
171 which forwards to [Gitiles][cr-gitiles], rather than linking to the review at
172 <https://chromium-review.googlesource.com>.
173
174- If there are instructions for testers to verify the change is correct,
175 include them with the `Test:` tag:
176
177 ```
178 Test: Load example.com/page.html and click the foo-button; see
179 crbug.com/123456 for more details.
180 ```
181
182After saving the change description, `git-cl` runs some presubmit scripts to
183check for common errors. If everything passes, `git-cl` will print something
184like this:
185
186```
187remote: SUCCESS
188remote:
189remote: New Changes:
190remote: https://chromium-review.googlesource.com/c/chromium/src/+/1485699 Use base::TimeDelta::FromTimeSpec helper in more places. [WIP]
191```
192
193Additional flags can be used to specify reviewers, bugs fixed by the change, et
194cetera:
195
196```
197git cl upload -r [email protected],[email protected] -b 123456
198```
199
200See `git cl help upload` for a full list of flags.
201
202## Code review
203
204Code reviews are covered in more detail on the [code review
205policies][code-reviews] page.
206
207### Finding a reviewer
208
209Ideally, the reviewer is someone who is familiar with the area of code in
210question. If you're not sure who that should be, check with anyone in the
211nearest ancestor OWNERS file.
212
213- Anybody can review code, but there must be at least one owner for each
214 affected directory.
215- If there are multiple reviewers, make it clear what each reviewer is expected
216 to review. Otherwise, people might assume their input is not required or
217 waste time with redundant reviews.
218- `git cl owners` automatically suggests reviewers based on the OWNERS files.
219
220### Requesting review
221
222Open the change on [the web][crrev]. If you can't find the link, running `git
223cl issue` will display the review URL for the current branch. Alternatively,
224visit <https://chromium-review.googlesource.com> and look in the "Outgoing
225Reviews" section.
226
227Reviewers expect to review code that compiles and passes tests. If you have
228access, now is a good time to run your change through the [automated
229tests](#running-automated-tests).
230
231Click **Add Reviewers** in the left column (if you don't see this link, make
232sure you are logged in). In the **Reviewers** field, enter a comma-separated
233list of the reviewers you picked.
234
235In the same dialog, you can include an optional message to your reviewers. This
236space can be used for specific questions or instructions. Once you're done,
Fredrik Söderquist53031aff2020-02-07 16:04:00237make sure to click **Start Review**, which notifies the requested reviewers that
238they should review your change.
Daniel Cheng86135f32019-02-27 07:10:38239
240**IMPORTANT: UNTIL YOU SEND THE REVIEW REQUEST, NO ONE WILL LOOK AT THE REVIEW**
241
242### Review process
243
244All changes must be reviewed (see [code review policies][code-reviews]).
245
246You should get a response within **one** business day; re-ping your reviewers
247if you do not.
248
249To upload new patch sets that address comments from the reviewers, simply
250commit more changes to your local branch and run `git cl upload` again.
251
252### Approval
253
254When the reviewer is happy with the change, they will set the "Code-Review +1"
255label. Owners of all affected files must approve before a change can be
256committed. See: [code review policies: owners][code-reviews-owners].
257
258## Running automated tests
259
260Before being submitted, a change must pass the commit queue (CQ). The commit
261queue is an automated system which sends a patch to multiple try bots running
262different platforms: each try bot compiles Chromium with the patch and ensures
263the tests still pass on that platform.
264
265To trigger this process, click **CQ Dry Run** in the upper right corner of the
266code review tool. Note that this is equivalent to setting the "Commit-Queue +1"
267label. Anyone can set this label; however, the CQ will not process the patch
268unless the person setting the label has [try job access][try-job-access].
269
270If you don't have try job access and:
271
272- you have an @chromium.org email address, request access for yourself.
273- you have contributed a few patches, ask a reviewer to nominate you for access.
274- neither of the above is true, request that a reviewer run try jobs for you in
275 the code review request message.
276
277The status of the latest try job for a given patchset is visible just below the
278list of changed files. Each bot has its own bubble, using one of the following
279colors to indicate its status:
280
281- Gray: the bot has not started processing the patch yet.
282- Yellow: the run is in progress. Check back later!
283- Purple: the trybot encountered an exception while processing the patch.
284 Usually, this is not the fault of the patch. Try clicking **CQ Dry Run**
285 again.
286- Red: tests failed. Click on the failed bot to see what tests failed and why.
287- Green: the run passed!
288
289## Committing
290
291Changes should generally be committed via the [commit queue][commit-queue].
292This is done by clicking **Submit to CQ** in the upper right corner, or setting
293the "Commit-Queue +2" label on the change. The commit queue will then
294send the patch to the try bots. If all try bots return green, the change will
295automatically be committed. Yay!
296
297Sometimes a test might be flaky. If you have an isolated failure that appears
298unrelated to your change, try sending the change to the commit queue again.
299
300Alternatively, a developer with commit access can [directly
301commit][direct-commit] a change, bypassing the commit queue. This should only
302be used in emergencies because it will bypass all the safety nets.
303
Darin Fisher0e196ee82019-09-06 22:39:12304## Code guidelines
305
306In addition to the adhering to the [styleguide][cr-styleguide], the following
307general rules of thumb can be helpful in navigating how to structure changes:
308
Darin Fisherf061fb12019-11-15 23:46:13309- **Code in the Chromium project should be in service of other code in the
310 Chromium project.** This is important so developers can understand the
311 constraints informing a design decision. Those constraints should be apparent
312 from the scope of code within the boundary of the project and its various
313 repositories. In other words, for each line of code, you should be able to
314 find a product in the Chromium repositories that depends on that line of code
315 or else the line of code should be removed.
Darin Fisher0e196ee82019-09-06 22:39:12316
317- **Code should only be moved to a central location (e.g., //base) when
318 multiple consumers would benefit.** We should resist the temptation to
319 build overly generic common libraries as that can lead to code bloat and
320 unnecessary complexity in common code.
321
322- **The code likely wasn't designed for everything we are trying to do with it
323 now.** Take time to refactor existing code to make sure the new feature or
324 subcomponent you are developing fits properly within the system. Technical
325 debt is easy to accumulate and is everyone's responsibility to avoid.
326
327- **Common code is everyone's responsibility.** Large files that are at the
328 cross-roads of many subsystems, where integration happens, can be some of the
329 most fragile in the system. As a companion to the previous point, be
330 cognizant of how you may be adding more complexity to the commons as you
331 venture to complete your task.
332
333- **Changes should include corresponding tests.** Automated testing is at the
334 heart of how we move forward as a project. All changes should include
335 corresponding tests so we can ensure that there is good coverage for code and
336 that future changes will be less likely to regress functionality. Protect
337 your code with tests!
338
Daniel Cheng86135f32019-02-27 07:10:38339## Tips
340
Dominik Röttschesd113bfa2019-07-10 08:56:24341### Review etiquette
342
Daniel Cheng86135f32019-02-27 07:10:38343During the lifetime of a review, you may want to rebase your change onto a newer
344source revision to minimize merge conflicts. The reviewer-friendly way to do
345this is to first address any unresolved comments and upload those changes as a
346patchset. Then, rebase to the newer revision and upload that as its own
347patchset (with no other changes). This makes it easy for reviewers to see the
348changes made in response to their comments, and then quickly verify the diffs
349from the rebase.
350
351Code authors and reviewers should keep in mind that Chromium is a global
352project: contributors and reviewers are often in time zones far apart. Please
353read these guidelines on [minimizing review lag][review-lag] and take them in
354consideration both when writing reviews and responding to review feedback.
355
Dominik Röttschesd113bfa2019-07-10 08:56:24356### Watchlists
357
358If you would like to be notified about changes to a set of files covering a
359topic or an area of Chromium, you may use the [watchlists][watchlist-doc]
360feature in order to receive email notifications.
361
362
Daniel Cheng86135f32019-02-27 07:10:38363[//]: # (the reference link section should be alphabetically sorted)
364[checkout-and-build]: https://chromium.googlesource.com/chromium/src/+/master/docs/#checking-out-and-building
John Abd-El-Malek27e1cf02019-12-18 17:35:18365[chrome-dd-review-process]: http://go/chrome-dd-review-process
366[chromium-design-docs]: https://groups.google.com/a/chromium.org/forum/#!forum/chromium-design-docs
Daniel Cheng86135f32019-02-27 07:10:38367[cl-footer-syntax]: https://dev.chromium.org/developers/contributing-code/-bug-syntax
368[code-reviews-owners]: code_reviews.md#OWNERS-files
369[code-reviews]: code_reviews.md
Toby Huang5105f812019-08-08 23:47:57370[commit-checklist]: commit_checklist.md
Daniel Cheng86135f32019-02-27 07:10:38371[commit-queue]: infra/cq.md
372[core-principles]: https://www.chromium.org/developers/core-principles
373[corporate-cla]: https://cla.developers.google.com/about/google-corporate?csw=1
374[cr-authors]: https://chromium.googlesource.com/chromium/src/+/HEAD/AUTHORS
375[cr-gitiles]: https://chromium.googlesource.com/chromium/src/+/master/
376[cr-styleguide]: https://chromium.googlesource.com/chromium/src/+/master/styleguide/styleguide.md
377[crbug-new]: https://bugs.chromium.org/p/chromium/issues/entry
378[crbug]: https://bugs.chromium.org/p/chromium/issues/list
379[cros-authors]: https://chromium.googlesource.com/chromium/src/+/master/AUTHORS
380[cros-dev-guide]: https://chromium.googlesource.com/chromiumos/docs/+/master/developer_guide.md
381[crrev]: https://chromium-review.googlesource.com
382[depot-tools-setup]: https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/depot_tools_tutorial.html#_setting_up
John Abd-El-Malek27e1cf02019-12-18 17:35:18383[design-doc-template]: https://docs.google.com/document/d/14YBYKgk-uSfjfwpKFlp_omgUq5hwMVazy_M965s_1KA
Daniel Cheng86135f32019-02-27 07:10:38384[direct-commit]: https://dev.chromium.org/developers/contributing-code/direct-commit
385[discussion-groups]: https://www.chromium.org/developers/discussion-groups
386[github-tutorial]: https://try.github.io
387[good-git-commit-message]: https://chris.beams.io/posts/git-commit/
388[individual-cla]: https://cla.developers.google.com/about/google-individual?csw=1
389[life-of-a-chromium-developer]: https://docs.google.com/a/google.com/present/view?id=0AetfwCoL2lQAZGQ5bXJ0NDVfMGRtdGQ0OWM2
390[noms-tutorial]: https://meowni.ca/posts/chromium-101
391[review-lag]: https://dev.chromium.org/developers/contributing-code/minimizing-review-lag-across-time-zones
392[skia-dev-guide]: https://skia.org/dev/contrib
393[try-job-access]: https://www.chromium.org/getting-involved/become-a-committer#TOC-Try-job-access
394[v8-dev-guide]: https://v8.dev/docs
Dominik Röttschesd113bfa2019-07-10 08:56:24395[watchlist-doc]: infra/watchlists.md