diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 53274f0ede..70d9ba267f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,4 +1,4 @@ -# Contributing +# How to Contribute to crosvm ## How to report bugs @@ -118,67 +118,6 @@ same tests locally. When all tests pass, your change is merged into `origin/main`. -## Philosophy - -The following is high level guidance for producing contributions to crosvm. - -- Prefer mechanism to policy. -- Use existing protocols when they are adequate, such as virtio. -- Prefer security over code re-use and speed of development. -- Only the version of Rust in use by the ChromeOS toolchain is supported. This is ordinarily the - stable version of Rust, but can be behind a version for a few weeks. -- Avoid distribution specific code. - -## Style guidelines - -### Formatting - -To format all code, crosvm defers to `rustfmt`. In addition, the code adheres to the following -rules: - -Each `use` statement should import a single item, as produced by `rustfmt` with -[`imports_granularity=item`]. Do not use braces to import multiple items. - -The `use` statements for each module should be grouped into blocks separated by whitespace in the -order produced by `rustfmt` with [`group_imports=StdExternalCrate`] and sorted alphabetically: - -1. `std` -1. third-party + crosvm crates -1. `crate` + `super` - -The import formatting options of `rustfmt` are currently unstable, so these are not enforced -automatically. If a nightly Rust toolchain is present, it is possible to automatically reformat the -code to match these guidelines by running `tools/fmt --nightly`. - -crosvm uses the [remain](https://github.com/dtolnay/remain) crate to keep error enums sorted, along -with the `#[sorted]` attribute to keep their corresponding match statements in the same order. - -### Unit test code - -Unit tests and other highly-specific tests (which may include some small, but not all, integration -tests) should be written differently than how non-test code is written. Tests prevent regressions -from being committed, show how APIs can be used, and help with understanding bugs in code. That -means tests must be clear both now and in the future to a developer with low familiarity of the code -under test. They should be understandable by reading from top to bottom without referencing any -other code. Towards these goals, tests should: - -- To a reasonable extent, be structured as Arrange-Act-Assert. -- Test the minimum number of behaviors in a single test. Make separate tests for separate behavior. -- Avoid helper methods that send critical inputs or assert outputs within the helper itself. It - should be easy to read a test and determine the critical inputs/outputs without digging through - helper methods. Setup common to many tests is fine to factor out, but lean toward duplicating code - if it aids readability. -- Avoid branching statements like conditionals and loops (which can make debugging more difficult). -- Document the reason constants were chosen in the test, including if they were picked arbitrarily - such that in the future, changing the value is okay. (This can be done with constant variable - names, which is ideal if the value is used more than once, or in a comment.) -- Name tests to describe what is being tested and the expected outcome, for example - `test_foo_invalid_bar_returns_baz`. - -Less-specific tests, such as most integration tests and system tests, are more likely to require -obfuscating work behind helper methods. It is still good to strive for clarity and ease of debugging -in those tests, but they do not need to follow these guidelines. - ## Contributing to the documentation [The book of crosvm] is built with [mdBook]. Each markdown file must follow @@ -201,5 +140,3 @@ Output is found at `docs/book/book/html/`. [mdbook]: https://rust-lang.github.io/mdBook/ [mdbook-mermaid]: https://github.com/badboy/mdbook-mermaid [the book of crosvm]: https://crosvm.dev/book/ -[`group_imports=stdexternalcrate`]: https://rust-lang.github.io/rustfmt/?version=v1.5.1&search=#group_imports -[`imports_granularity=item`]: https://rust-lang.github.io/rustfmt/?version=v1.5.1&search=#imports_granularity diff --git a/docs/book/src/SUMMARY.md b/docs/book/src/SUMMARY.md index 0962560e03..197e25664e 100644 --- a/docs/book/src/SUMMARY.md +++ b/docs/book/src/SUMMARY.md @@ -26,7 +26,9 @@ - [ChromeOS](./integration/chromeos.md) - [Architecture](./architecture.md) - [Hypervisors](./hypervisors.md) -- [Contributing](./contributing/index.md) +- [Contribution Guide](./contributing/index.md) + - [Contributing to crosvm](./contributing/contributing.md) + - [Coding Style](./contributing/coding_style.md) - [Style Guide for Platform-Specific Code](./contributing/style_guide_platform_specific_code.md) ______________________________________________________________________ diff --git a/docs/book/src/contributing/coding_style.md b/docs/book/src/contributing/coding_style.md new file mode 100644 index 0000000000..1883948447 --- /dev/null +++ b/docs/book/src/contributing/coding_style.md @@ -0,0 +1,65 @@ +# Coding Style Guide + +## Philosophy + +The following is high level guidance for producing contributions to crosvm. + +- Prefer mechanism to policy. +- Use existing protocols when they are adequate, such as virtio. +- Prefer security over code re-use and speed of development. +- Only the version of Rust in use by the ChromeOS toolchain is supported. This is ordinarily the + stable version of Rust, but can be behind a version for a few weeks. +- Avoid distribution specific code. + +## Style guidelines + +### Formatting + +To format all code, crosvm defers to `rustfmt`. In addition, the code adheres to the following +rules: + +Each `use` statement should import a single item, as produced by `rustfmt` with +[`imports_granularity=item`]. Do not use braces to import multiple items. + +The `use` statements for each module should be grouped into blocks separated by whitespace in the +order produced by `rustfmt` with [`group_imports=StdExternalCrate`] and sorted alphabetically: + +1. `std` +1. third-party + crosvm crates +1. `crate` + `super` + +The import formatting options of `rustfmt` are currently unstable, so these are not enforced +automatically. If a nightly Rust toolchain is present, it is possible to automatically reformat the +code to match these guidelines by running `tools/fmt --nightly`. + +crosvm uses the [remain](https://github.com/dtolnay/remain) crate to keep error enums sorted, along +with the `#[sorted]` attribute to keep their corresponding match statements in the same order. + +### Unit test code + +Unit tests and other highly-specific tests (which may include some small, but not all, integration +tests) should be written differently than how non-test code is written. Tests prevent regressions +from being committed, show how APIs can be used, and help with understanding bugs in code. That +means tests must be clear both now and in the future to a developer with low familiarity of the code +under test. They should be understandable by reading from top to bottom without referencing any +other code. Towards these goals, tests should: + +- To a reasonable extent, be structured as Arrange-Act-Assert. +- Test the minimum number of behaviors in a single test. Make separate tests for separate behavior. +- Avoid helper methods that send critical inputs or assert outputs within the helper itself. It + should be easy to read a test and determine the critical inputs/outputs without digging through + helper methods. Setup common to many tests is fine to factor out, but lean toward duplicating code + if it aids readability. +- Avoid branching statements like conditionals and loops (which can make debugging more difficult). +- Document the reason constants were chosen in the test, including if they were picked arbitrarily + such that in the future, changing the value is okay. (This can be done with constant variable + names, which is ideal if the value is used more than once, or in a comment.) +- Name tests to describe what is being tested and the expected outcome, for example + `test_foo_invalid_bar_returns_baz`. + +Less-specific tests, such as most integration tests and system tests, are more likely to require +obfuscating work behind helper methods. It is still good to strive for clarity and ease of debugging +in those tests, but they do not need to follow these guidelines. + +[`group_imports=stdexternalcrate`]: https://rust-lang.github.io/rustfmt/?version=v1.5.1&search=#group_imports +[`imports_granularity=item`]: https://rust-lang.github.io/rustfmt/?version=v1.5.1&search=#imports_granularity diff --git a/docs/book/src/contributing/contributing.md b/docs/book/src/contributing/contributing.md new file mode 100644 index 0000000000..a3ac16626d --- /dev/null +++ b/docs/book/src/contributing/contributing.md @@ -0,0 +1 @@ +{{#include ../../../../CONTRIBUTING.md}} diff --git a/docs/book/src/contributing/index.md b/docs/book/src/contributing/index.md index a3ac16626d..66e4240bdb 100644 --- a/docs/book/src/contributing/index.md +++ b/docs/book/src/contributing/index.md @@ -1 +1,9 @@ -{{#include ../../../../CONTRIBUTING.md}} +# Contributing to crosvm + +This chapter provides information for those who want to contribute to the crosvm. + +- [How to contribute](./contributing.md) - General guideline to contributing to crosvm, including + reporting a bug, sending a patch, and updating this documentation. +- [Coding Style](./coding_style.md) - Coding style guide for crosvm. +- [Style Guide for Platform-Specific Code](./style_guide_platform_specific_code.md) - Guideline to + write platform-specific code cleanly in crosvm.