|
| 1 | + |
| 2 | +# Coding standards |
| 3 | + |
| 4 | +Generally we just follow good sensible Rust practices, clippy and so forth. |
| 5 | +However there are some practices we've agreed on that are not machine-enforced; |
| 6 | +meeting those requirements in a PR will make it easier to merge. |
| 7 | + |
| 8 | +## Import grouping |
| 9 | + |
| 10 | +In each file the imports should be grouped into at most 4 groups in the |
| 11 | +following order: |
| 12 | + |
| 13 | +1. stdlib |
| 14 | +2. non-repository local crates |
| 15 | +3. repository local other crates |
| 16 | +4. this crate |
| 17 | + |
| 18 | +Separate each group with a blank line, and rustfmt will sort into a canonical |
| 19 | +order. Any file that is not grouped like this can be rearranged whenever the |
| 20 | +file is touched - we're not precious about having it done in a separate commit, |
| 21 | +though that is helpful. |
| 22 | + |
| 23 | +## No direct use of process state outside rustup::currentprocess |
| 24 | + |
| 25 | +The `rustup::currentprocess` module abstracts the global state that is |
| 26 | +`std::env::args`, `std::env::vars`, `std::io::std*`, `std::process::id`, |
| 27 | +`std::env::current_dir` and `std::process::exit` permitting threaded tests of |
| 28 | +the CLI logic; use `process()` rather than those APIs directly. |
| 29 | + |
| 30 | +## Clippy lints |
| 31 | + |
| 32 | +We do not enforce lint status in the checks done by GitHub Actions, because |
| 33 | +clippy is a moving target that can make it hard to merge for little benefit. |
| 34 | + |
| 35 | +We do ask that contributors keep the clippy status clean themselves. |
| 36 | + |
| 37 | +Minimally, run `cargo +beta clippy --all --all-targets -- -D warnings` before |
| 38 | +submitting code. |
| 39 | + |
| 40 | +If possible, adding `--all-features` to the command is useful, but will require |
| 41 | +additional dependencies like `libcurl-dev`. |
| 42 | + |
| 43 | +Regular contributors or contributors to particularly OS-specific code should |
| 44 | +also make sure that their clippy checking is done on at least Linux and Windows, |
| 45 | +as OS-conditional code is a common source of unused imports and other small |
| 46 | +lints, which can build up over time. |
| 47 | + |
| 48 | +For developers using BSD/Linux/Mac OS, there are Windows VM's suitable for such |
| 49 | +development tasks for use with virtualbox and other hypervisors are downloadable |
| 50 | +from |
| 51 | +[Microsoft](https://developer.microsoft.com/en-us/windows/downloads/virtual-machines/). |
| 52 | +Similarly, there are many Linux and Unix operating systems images available for |
| 53 | +developers whose usual operating system is Windows. Currently Rustup has no Mac |
| 54 | +OS specific code, so there should be no need to worry about Mac VM images. |
| 55 | + |
| 56 | +Clippy is also run in GitHub Actions, in the `General Checks / Checks` build |
| 57 | +task, but not currently run per-platform, which means there is no way to find |
| 58 | +out the status of clippy per platform without running it on that platform as a |
| 59 | +developer. |
| 60 | + |
| 61 | +### import rustup-macros::{integration,unit}_test into test modules |
| 62 | + |
| 63 | +These test helpers add pre-and-post logic to tests to enable the use of tracing |
| 64 | +inside tests, which can be helpful for tracking down behaviours in larger tests. |
0 commit comments