xref: /aosp_15_r20/external/crosvm/CONTRIBUTING.md (revision bb4ee6a4ae7042d18b07a98463b9c8b875e44b39)
1# How to Contribute to crosvm
2
3## How to report bugs
4
5We use Google issue tracker. Please use
6[the public crosvm component](https://issuetracker.google.com/issues?q=status:open%20componentid:1161302).
7
8**For Googlers**: See [go/crosvm#filing-bugs](https://goto.google.com/crosvm#filing-bugs).
9
10## Contributing code
11
12### Gerrit Account
13
14You need to set up a user account with [gerrit](https://chromium-review.googlesource.com/). Once
15logged in, you can obtain
16[HTTP Credentials](https://chromium-review.googlesource.com/settings/#HTTPCredentials) to set up git
17to upload changes.
18
19Once set up, run `./tools/cl` to install the gerrit commit message hook. This will insert a unique
20"Change-Id" into all commit messages so gerrit can identify changes. Even warning messages appear,
21the message hook will be installed.
22
23### Contributor License Agreement
24
25Contributions to this project must be accompanied by a Contributor License Agreement (CLA). You (or
26your employer) retain the copyright to your contribution; this simply gives us permission to use and
27redistribute your contributions as part of the project. Head over to
28<https://cla.developers.google.com/> to see your current agreements on file or to sign a new one.
29
30You generally only need to submit a CLA once, so if you've already submitted one (even if it was for
31a different project), you probably don't need to do it again.
32
33### Commit Messages
34
35As for commit messages, we follow
36[ChromeOS's guideline](https://www.chromium.org/chromium-os/developer-library/guides/development/contributing/#commit-messages)
37in general.
38
39Here is an example of a good commit message:
40
41```
42devices: vhost: user: vmm: Add Connection type
43
44This abstracts away the cross-platform differences:
45cfg(any(target_os = "android", target_os = "linux")) uses a Unix
46domain domain stream socket to connect to the vhost-user backend, and
47cfg(windows) uses a Tube.
48
49BUG=b:249361790
50TEST=tools/presubmit --all
51
52Change-Id: I47651060c2ce3a7e9f850b7ed9af8bd035f82de6
53```
54
55- The first line is a subject that starts with a tag that represents which components your commit
56  relates to. Tags are usually the name of the crate you modified such as `devices:` or `base:`. If
57  you only modified a specific component in a crate, you can specify the path to the component as a
58  tag like `devices: vhost: user:`. If your commit modified multiple crates, specify the crate where
59  your main change exists. The subject should be no more than 50 characters, including any tags.
60- The body should consist of a motivation followed by an impact/action. The body text should be
61  wrapped to 72 characters.
62- `BUG` lines are used to specify an associated issue number. If the issue is filed at
63  [Google's issue tracker](https://issuetracker.google.com/), write `BUG=b:<bug number>`. If no
64  issue is associated, write `BUG=None`. You can have multiple `BUG` lines.
65- `TEST` lines are used to describe how you tested your commit in a free form. You can have multiple
66  `TEST` lines.
67- `Change-Id` is used to identify your change on Gerrit. It's inserted by the gerrit commit message
68  hook as explained in
69  [the previous section](https://crosvm.dev/book/contributing/index.html#gerrit-account). If a new
70  commit is uploaded with the same `Change-Id` as an existing CL's `Change-Id`, gerrit will
71  recognize the new commit as a new patchset of the existing CL.
72
73### Uploading changes
74
75To make changes to crosvm, start your work on a new branch tracking `origin/main`.
76
77```bash
78git checkout -b myfeature --track origin/main
79```
80
81After making the necessary changes, and testing them via
82[Presubmit Checks](https://crosvm.dev/book/building_crosvm/linux.html#presubmit-checks), you can
83commit and upload them:
84
85```bash
86git commit
87./tools/cl upload
88```
89
90If you need to revise your change, you can amend the existing commit and upload again:
91
92```bash
93git commit --amend
94./tools/cl upload
95```
96
97This will create a new version of the same change in gerrit.
98
99If the branch contains multiple commits, each one will be uploaded as a separate review, and they
100will be linked in Gerrit as [related changes]. You may revise any commit in a branch using tools
101like `git rebase` and then re-upload the whole series with `./tools/cl upload` when `HEAD` is
102pointing to the tip of the branch.
103
104> Note: We don't accept any pull requests on the [GitHub mirror].
105
106### Getting Reviews
107
108All submissions needs to be reviewed by one of the [crosvm owners]. Use the gerrit UI to request a
109review and add [email protected] to assign to a random owner.
110
111If you run into issues with reviews, reach out to the team via
112[chat](https://matrix.to/#/#crosvm:matrix.org) or
113[email list](https://groups.google.com/a/chromium.org/g/crosvm-dev).
114
115**For Googlers**: see [go/crosvm-chat](https://goto.google.com/crosvm-chat).
116
117#### Any change to Cargo.lock
118
119When adding a new crate from crates.io, additional review is required to ensure that the crate meets
120the crosvm project standards. This review is provided by the members of `OWNERS_COUNCIL`.
121
122Unfortunately, our tooling cannot tell the difference between adding an external crate and changing
123dependencies within crosvm (e.g. `devices` depending on a new internal crosvm utility crate). For
124those cases, a rubberstamp is still needed from `OWNERS_COUNCIL`.
125
126**For Googlers**: see [go/crosvm/3p_crates](https://goto.google.com/crosvm/3p_crates).
127
128### Reviewing code (for OWNERS)
129
130We have two major types of reviewers on the project:
131
1321. Global OWNERS: these folks are broadly responsible for the health of the crosvm project, and have
133   expertise in multiple project subdomains. While they can technically approve any change, they
134   will often delegate to area OWNERS when a change is outside their expertise.
1351. Area OWNERS: experts in a particular subdomain of the project (e.g. graphics, USB, etc). Major
136   changes in an area SHOULD be reviewed by an area OWNER, if one exists (not all subdomains have
137   OWNERS).
138
139All owners are expected to review code in their areas, and to aim for the following goals in
140reviews:
141
142- Reply to reviews within 1 working day. If this is infeasible (especially if overloaded), reassign
143  to crosvm-reviews@ to pick another OWNER at random.
144- Defer to the [styleguide](./coding_style.md) where it makes sense to do so. Update the styleguide
145  when it does not.
146- Strive to avoid reviews getting stuck in endless back & forth. If you see this happening, you can:
147  - Schedule a meeting to discuss it online. Consider inviting another OWNER to help brainstorm
148    solutions.
149  - Bring the review discussion to the hallway chat to let the group weigh in.
150- Follow generally accepted practices for good code review
151  - Technically: We insist on good documentation, clean APIs especially when broadly consumed, and
152    generally keep code health in mind.
153  - Socially: Our goal, above all else, is to be good peers to each other. So we review *code*, not
154    *authors*. We remember to disagree respectfully, and that a code review is a team effort (author
155    and reviewer) against a hard technical problem.
156
157### Submitting code
158
159Crosvm uses a Commit Queue, which will run pre-submit testing on all changes before merging them
160into crosvm.
161
162Once one of the [crosvm owners] has voted "Code-Review+2" on your change, you can use the "Submit to
163CQ" button, which will trigger the test process.
164
165Gerrit will show any test failures. Refer to
166[Building Crosvm](https://crosvm.dev/book/building_crosvm/) for information on how to run the same
167tests locally.
168
169Each individual change in a patch series must build and pass the tests. If you are working on a
170series of related changes, ensure that each incremental commit does not cause test regressions or
171break the build if it is merged without the later changes in the series. For example, an
172intermediate change must not trigger any unused code warnings or cause test failures that are fixed
173by later changes in the series.
174
175When all tests pass, your change is merged into `origin/main`.
176
177## Contributing to the documentation
178
179[The book of crosvm] is built with [mdBook]. Each markdown file must follow
180[Google Markdown style guide].
181
182To render the book locally, you need to install mdbook and [mdbook-mermaid], which should be
183installed when you run `./tools/install-deps` script. Or you can use the `tools/dev_container`
184environment.
185
186```sh
187cd docs/book/
188mdbook build
189```
190
191Output is found at `docs/book/book/html/`.
192
193To format markdown files, run `./tools/fmt` in the `dev_container`.
194
195[crosvm owners]: https://chromium.googlesource.com/crosvm/crosvm/+/HEAD/OWNERS
196[github mirror]: https://github.com/google/crosvm
197[google markdown style guide]: https://github.com/google/styleguide/blob/gh-pages/docguide/style.md
198[mdbook]: https://rust-lang.github.io/mdBook/
199[mdbook-mermaid]: https://github.com/badboy/mdbook-mermaid
200[related changes]: https://gerrit-review.googlesource.com/Documentation/user-review-ui.html#related-changes
201[the book of crosvm]: https://crosvm.dev/book/
202