aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--docs/dev/README.md105
1 files changed, 104 insertions, 1 deletions
diff --git a/docs/dev/README.md b/docs/dev/README.md
index 65cc9fc12..1de5a2aab 100644
--- a/docs/dev/README.md
+++ b/docs/dev/README.md
@@ -30,7 +30,7 @@ https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0
30 30
31* [good-first-issue](https://github.com/rust-analyzer/rust-analyzer/labels/good%20first%20issue) 31* [good-first-issue](https://github.com/rust-analyzer/rust-analyzer/labels/good%20first%20issue)
32 are good issues to get into the project. 32 are good issues to get into the project.
33* [E-mentor](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-mentor) 33* [E-has-instructions](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-has-instructions)
34 issues have links to the code in question and tests. 34 issues have links to the code in question and tests.
35* [E-easy](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-easy), 35* [E-easy](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-easy),
36 [E-medium](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-medium), 36 [E-medium](https://github.com/rust-analyzer/rust-analyzer/issues?q=is%3Aopen+is%3Aissue+label%3AE-medium),
@@ -117,6 +117,109 @@ Additionally, I use `cargo run --release -p rust-analyzer -- analysis-stats
117path/to/some/rust/crate` to run a batch analysis. This is primarily useful for 117path/to/some/rust/crate` to run a batch analysis. This is primarily useful for
118performance optimizations, or for bug minimization. 118performance optimizations, or for bug minimization.
119 119
120# Code Style & Review Process
121
122Our approach to "clean code" is two fold:
123
124* We generally don't block PRs on style changes.
125* At the same time, all code in rust-analyzer is constantly refactored.
126
127It is explicitly OK for reviewer to flag only some nits in the PR, and than send a follow up cleanup PR for things which are easier to explain by example, cc-ing the original author.
128Sending small cleanup PRs (like rename a single local variable) is encouraged.
129
130## Scale of Changes
131
132Everyone knows that it's better to send small & focused pull requests.
133The problem is, sometimes you *have* to, eg, rewrite the whole compiler, and that just doesn't fit into a set of isolated PRs.
134
135The main thing too keep an eye on is the boundaries between various components.
136There are three kinds of changes:
137
1381. Internals of a single component are changed.
139 Specifically, you don't change any `pub` items.
140 A good example here would be an addition of a new assist.
141
1422. API of a component is expanded.
143 Specifically, you add a new `pub` function which wasn't there before.
144 A good example here would be expansion of assist API, for example, to implement lazy assists or assists groups.
145
1463. A new dependency between components is introduced.
147 Specifically, you add a `pub use` reexport from another crate or you add a new line to `[dependencies]` section of `Cargo.toml`.
148 A good example here would be adding reference search capability to the assists crates.
149
150For the first group, the change is generally merged as long as:
151
152* it works for the happy case,
153* it has tests,
154* it doesn't panic for unhappy case.
155
156For the second group, the change would be subjected to quite a bit of scrutiny and iteration.
157The new API needs to be right (or at least easy to change later).
158The actual implementation doesn't matter that much.
159It's very important to minimize the amount of changed lines of code for changes of the second kind.
160Often, you start doing change of the first kind, only to realise that you need to elevate to a change of the second kind.
161In this case, we'll probably ask you to split API changes into a separate PR.
162
163Changes of the third group should be pretty rare, so we don't specify any specific process for them.
164That said, adding an innocent-looking `pub use` is a very simple way to break encapsulation, keep an eye on it!
165
166Note: if you enjoyed this abstract hand-waving about boundaries, you might appreciate
167https://www.tedinski.com/2018/02/06/system-boundaries.html
168
169## Order of Imports
170
171We separate import groups with blank lines
172
173```
174mod x;
175mod y;
176
177use std::{ ... }
178
179use crate_foo::{ ... }
180use crate_bar::{ ... }
181
182use crate::{}
183
184use super::{} // but prefer `use crate::`
185```
186
187## Order of Items
188
189Optimize for the reader who sees the file for the first time, and wants to get the general idea about what's going on.
190People read things from top to bottom, so place most important things first.
191
192Specifically, if all items except one are private, always put the non-private item on top.
193
194Put `struct`s and `enum`s first, functions and impls last.
195
196Do
197
198```
199// Good
200struct Foo {
201 bars: Vec<Bar>
202}
203
204struct Bar;
205```
206
207rather than
208
209```
210// Not as good
211struct Bar;
212
213struct Foo {
214 bars: Vec<Bar>
215}
216```
217
218## Documentation
219
220For `.md` and `.adoc` files, prefer a sentence-per-line format, don't wrap lines.
221If the line is too long, you want to split the sentence in two :-)
222
120# Logging 223# Logging
121 224
122Logging is done by both rust-analyzer and VS Code, so it might be tricky to 225Logging is done by both rust-analyzer and VS Code, so it might be tricky to