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