Skip to content

Commit 5a46025

Browse files
committed
Auto merge of #14222 - epage:contrib, r=weihanglo
docs(contrib): Document things I look for in RFCs ### What does this PR try to resolve? Trying to make the RFC experience more pleasant by being upfront about things they need to take into account, rather than waiting for Cargo team members to review it (which they can also miss considering these points). Along the way, I ended up tweaking the unstable docs as they are a related part of the process. ### How should we test and review this PR? ### Additional information
2 parents 1d9058f + 369b3ff commit 5a46025

File tree

4 files changed

+108
-33
lines changed

4 files changed

+108
-33
lines changed

src/doc/contrib/src/SUMMARY.md

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
- [Process](./process/index.md)
77
- [Working on Cargo](./process/working-on-cargo.md)
88
- [Release process](./process/release.md)
9+
- [Writing an RFC](./process/rfc.md)
910
- [Unstable features](./process/unstable.md)
1011
- [Security issues](./process/security.md)
1112
- [Design Principles](./design.md)

src/doc/contrib/src/process/index.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,6 @@ on how this works.
114114
[issue-feature-request]: https://github.com/rust-lang/cargo/labels/C-feature-request
115115
[Feature accepted]: https://github.com/rust-lang/cargo/labels/Feature%20accepted
116116
[design principles chapter]: ../design.md
117-
[RFC process]: https://github.com/rust-lang/rfcs/
117+
[RFC process]: ./rfc.md
118118
[irlo]: https://internals.rust-lang.org/
119119
[subcommands]: https://doc.rust-lang.org/cargo/reference/external-tools.html#custom-subcommands

src/doc/contrib/src/process/rfc.md

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# Writing an RFC
2+
3+
Generally, an RFC goes through:
4+
1. Pre-RFC discussions on the [internals forum][irlo]
5+
2. [RFC]
6+
3. [Development and stabilization][unstable]
7+
8+
Please keep in mind our [design principles](../design.md).
9+
10+
For more concrete areas of consideration:
11+
12+
## `.cargo/config.toml` and `Cargo.toml`
13+
14+
`.cargo/config.toml` is for environment or transient configuration,
15+
being dependent on what directory you are running from and settable on the command-line,
16+
independent of other flags like `--manifest-path` or `--package`.
17+
18+
On the other hand `Cargo.toml` is for static, high-level project configuration.
19+
20+
For example,
21+
- [RFC 3537] chose
22+
configuration for the MSRV-aware resolver because users would likely need
23+
to change this setting, like in CI to verify the opposite case of
24+
what they run by default.
25+
- The Cargo team rejected a [`[cfg]` table][cfg table] to represent `rustc`
26+
`--cfg` flags as it was a direct port of low-level rustc behavior that didn't
27+
mesh with the other high level abstractions of manifests.
28+
- For stabilization, this was worked around through a build script directive and a `[lints]` field configuration.
29+
- [#12738][cargo#12738] for exploring how existing config might be representable in `Cargo.toml`.
30+
31+
32+
[irlo]: https://internals.rust-lang.org/
33+
[RFC]: https://github.com/rust-lang/rfcs/
34+
[unstable]: unstable.md
35+
[RFC 3537]: https://rust-lang.github.io/rfcs/3537-msrv-resolver.html
36+
[cfg table]: https://github.com/rust-lang/cargo/pull/11631#issuecomment-1487424886
37+
[cargo#12738]: https://github.com/rust-lang/cargo/issues/12738
38+
39+
## `Cargo.toml`
40+
41+
When adding a table to a manifest,
42+
- Should it be inheritable?
43+
- Ensure the package table and the inheritable table under `workspace` align
44+
- Care is needed to ensure a `workspace = true` field doesn't conflict with other entries
45+
- e.g. [RFC 3389] had to explicitly exclude ever supporing a `workspace` linter
46+
47+
When adding a field,
48+
- Is it inheritable?
49+
- Consider whether sharing of the field would be driven by requirements or is a manifestion of the current implementation.
50+
For example, in most cases, dependency sources (e.g. `version` field) should be aligned across a workspace
51+
However, frequently dependency `features` will vary across a workspace.
52+
- When inheriting, can specify it in your package?
53+
- How does specifying a field in both `workspace` and a package interact?
54+
- e.g. dependency sources cannot be overridden
55+
- e.g. dependency `features` get merged
56+
- e.g. depedency `default-features` has been hard to get right ([#12162][cargo#12162])
57+
58+
When working extending `dependencies` tables:
59+
- How does this affect `cargo add` or `cargo remove`?
60+
- How does this affect `[patches]` which are just modified dependencies?
61+
62+
[RFC 3389]: https://rust-lang.github.io/rfcs/3389-manifest-lint.html
63+
[cargo#12162]: https://github.com/rust-lang/cargo/issues/12162
64+

src/doc/contrib/src/process/unstable.md

+42-32
Original file line numberDiff line numberDiff line change
@@ -5,27 +5,6 @@ feature will only be usable on the nightly channel, and requires a specific
55
opt-in by the user. Small changes can skip this process, but please consult
66
with the Cargo team first.
77

8-
## Unstable feature opt-in
9-
10-
For features that require behavior changes or new syntax in `Cargo.toml`, then
11-
it will need a `cargo-features` value placed at the top of `Cargo.toml` to
12-
enable it. The process for adding a new feature is described in the
13-
[`features` module]. Code that implements the feature will need to manually
14-
check that the feature is enabled for the current manifest.
15-
16-
For features that add new command-line flags, config options, or environment
17-
variables, then the `-Z` flags will be needed to enable them. The [`features`
18-
module] also describes how to add these. New flags should use the
19-
`fail_if_stable_opt` method to check if the `-Z unstable-options` flag has
20-
been passed.
21-
22-
## Unstable documentation
23-
24-
Every unstable feature should have a section added to the [unstable chapter]
25-
describing how to use the feature.
26-
27-
[unstable chapter]: https://github.com/rust-lang/cargo/blob/master/src/doc/src/reference/unstable.md
28-
298
## Tracking issues
309

3110
Each unstable feature should get a [tracking issue]. These issues are
@@ -58,20 +37,51 @@ something is only partially implemented, it may have both
5837
[S-accepted]: https://github.com/rust-lang/cargo/labels/S-accepted
5938
[S-waiting-on-feedback]: https://github.com/rust-lang/cargo/labels/S-waiting-on-feedback
6039

40+
## Implementation
41+
42+
See [Working on Cargo](working-on-cargo.md).
43+
44+
During implementation and testing, you may find reasons to deviate from the RFC.
45+
Please call these out in the tracking issue, with links to more information justifying the change
46+
(e.g. see [workspace inheritance tracking issue]).
47+
48+
[workspace inheritance tracking issue]: https://github.com/rust-lang/cargo/issues/8415
49+
50+
#### Unstable feature opt-in
51+
52+
For features that require behavior changes or new syntax in `Cargo.toml`, then
53+
it will need a `cargo-features` value placed at the top of `Cargo.toml` to
54+
enable it. The process for adding a new feature is described in the
55+
[`features` module]. Code that implements the feature will need to manually
56+
check that the feature is enabled for the current manifest.
57+
58+
For features that add new command-line flags, config options, or environment
59+
variables, then the `-Z` flags will be needed to enable them. The [`features`
60+
module] also describes how to add these. New flags should use the
61+
`fail_if_stable_opt` method to check if the `-Z unstable-options` flag has
62+
been passed.
63+
64+
#### Unstable documentation
65+
66+
Every unstable feature should have a section added to the [unstable chapter]
67+
describing how to use the feature.
68+
This can also serve as a place for the final documentation to live until its stabilized.
69+
70+
[unstable chapter]: https://github.com/rust-lang/cargo/blob/master/src/doc/src/reference/unstable.md
71+
6172
## Pre-Stabilization
6273

6374
Once an unstable feature is "complete", the search for users to test
64-
and give feedback begins. Testing notes should be written up to give users an
65-
idea of how to test the new feature. An example being the
66-
[workspace inheritance testing notes] for workspace inheritance. Once testing
67-
notes have been written up you should make posts in various rust communities
68-
([rust subreddit], [users], [internals], etc). Example posts made for workspace
69-
inheritance: [reddit post], [users post], [internals post]. The unstable feature
70-
should also be added to [This Week in Rust]. This should be done by adding the
71-
label `call-for-testing` to the RFC for the feature and making a comment with a
72-
link to the testing notes and the tracking issue (as needed). If there is not an
73-
RFC, a pull request should be made to the [TWiR repo] adding the feature to the
74-
`Call for Testing` section ([example]).
75+
and give feedback begins:
76+
1. Write up test instructions for users, summarizing where the feature is useful, how to use it (with links to the unstable documentation), and if there are any areas of particular concern
77+
- This could be on the tracking issue or in a dedicated issue for feedback
78+
- e.g. [workspace inheritance testing notes]
79+
2. Call for testing
80+
- In the RFC, link to the test instructions and label it with with `call-for-testing` to be picked up by [This Week in Rust]
81+
- If there is not an RFC, a pull request should be made to the [TWiR repo]
82+
adding the feature to the `Call for Testing` section ([example]).
83+
- Post on various Rust communities ([rust subreddit], [users], [internals], etc)
84+
- e.g. [reddit post], [users post], [internals post]
7585

7686
[workspace inheritance testing notes]: https://github.com/rust-lang/cargo/blob/6d6dd9d9be9c91390da620adf43581619c2fa90e/src/doc/src/reference/unstable.md#testing-notes
7787
[rust subreddit]: https://www.reddit.com/r/rust/

0 commit comments

Comments
 (0)