-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs(contrib): Document things I look for in RFCs #14222
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
0cda0f1
docs(contrib): Call out unstable docs being prep for stabilization
epage 7da5caa
docs(contrib): Make it easier to scan Call for Testing instructions
epage c9ed4be
docs(contrib): Tracking issues generally come before implementation
epage 5e9f4ad
docs(contrib): Group unstable impl sections
epage 248d272
docs(contrib): Link unstable dev to regular dev docs
epage e725a73
docs(contrib): Reasure people they can deviate from RFC
epage 369b3ff
docs(contrib): Note areas of consideration for RFCs
epage File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
# Writing an RFC | ||
|
||
Generally, an RFC goes through: | ||
1. Pre-RFC discussions on the [internals forum][irlo] | ||
2. [RFC] | ||
3. [Development and stabilization][unstable] | ||
|
||
Please keep in mind our [design principles](../design.md). | ||
|
||
For more concrete areas of consideration: | ||
|
||
## `.cargo/config.toml` and `Cargo.toml` | ||
|
||
`.cargo/config.toml` is for environment or transient configuration, | ||
being dependent on what directory you are running from and settable on the command-line, | ||
independent of other flags like `--manifest-path` or `--package`. | ||
|
||
On the other hand `Cargo.toml` is for static, high-level project configuration. | ||
|
||
For example, | ||
- [RFC 3537] chose | ||
configuration for the MSRV-aware resolver because users would likely need | ||
to change this setting, like in CI to verify the opposite case of | ||
what they run by default. | ||
- The Cargo team rejected a [`[cfg]` table][cfg table] to represent `rustc` | ||
`--cfg` flags as it was a direct port of low-level rustc behavior that didn't | ||
mesh with the other high level abstractions of manifests. | ||
- For stabilization, this was worked around through a build script directive and a `[lints]` field configuration. | ||
- [#12738][cargo#12738] for exploring how existing config might be representable in `Cargo.toml`. | ||
|
||
|
||
[irlo]: https://internals.rust-lang.org/ | ||
[RFC]: https://github.com/rust-lang/rfcs/ | ||
[unstable]: unstable.md | ||
[RFC 3537]: https://rust-lang.github.io/rfcs/3537-msrv-resolver.html | ||
[cfg table]: https://github.com/rust-lang/cargo/pull/11631#issuecomment-1487424886 | ||
[cargo#12738]: https://github.com/rust-lang/cargo/issues/12738 | ||
|
||
## `Cargo.toml` | ||
|
||
When adding a table to a manifest, | ||
- Should it be inheritable? | ||
- Ensure the package table and the inheritable table under `workspace` align | ||
- Care is needed to ensure a `workspace = true` field doesn't conflict with other entries | ||
- e.g. [RFC 3389] had to explicitly exclude ever supporing a `workspace` linter | ||
|
||
When adding a field, | ||
- Is it inheritable? | ||
- Consider whether sharing of the field would be driven by requirements or is a manifestion of the current implementation. | ||
For example, in most cases, dependency sources (e.g. `version` field) should be aligned across a workspace | ||
However, frequently dependency `features` will vary across a workspace. | ||
- When inheriting, can specify it in your package? | ||
- How does specifying a field in both `workspace` and a package interact? | ||
- e.g. dependency sources cannot be overridden | ||
- e.g. dependency `features` get merged | ||
- e.g. depedency `default-features` has been hard to get right ([#12162][cargo#12162]) | ||
|
||
When working extending `dependencies` tables: | ||
- How does this affect `cargo add` or `cargo remove`? | ||
- How does this affect `[patches]` which are just modified dependencies? | ||
|
||
[RFC 3389]: https://rust-lang.github.io/rfcs/3389-manifest-lint.html | ||
[cargo#12162]: https://github.com/rust-lang/cargo/issues/12162 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it just visual preference that we chose h4 instead of h3 here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, when browsing the site, h3 didn't seem to have enough differentiation from h2