docs(ref): Index differences between virtual / real manifests #13794
Conversation
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
rustbot
added
A-documenting-cargo-itself
weihanglo
reviewed
Apr 25, 2024
src/doc/src/reference/workspaces.md
Outdated
Comment on lines
83
to
90
| When there isn't a root package: | ||
|
|
||
| - [`resolver = "2"`](resolver.md#resolver-versions) must be | ||
| set explicitly in virtual workspaces as they have no | ||
| [`package.edition`][package-edition] to infer it from. | ||
| [resolver version](resolver.md#resolver-versions). | ||
| - Commands run in the workspace's directory will run against all workspace | ||
| members by default, see [Package selection](#package-selection). |
There was a problem hiding this comment.
Feel a bit disconnected with the previous paragraph. What about this:
Suggested change
| When there isn't a root package: | |
| - [`resolver = "2"`](resolver.md#resolver-versions) must be | |
| set explicitly in virtual workspaces as they have no | |
| [`package.edition`][package-edition] to infer it from. | |
| [resolver version](resolver.md#resolver-versions). | |
| - Commands run in the workspace's directory will run against all workspace | |
| members by default, see [Package selection](#package-selection). | |
| Some major differences bewteen root packages and virtual workspaces: | |
| - In virtual workspaces, [`resolver = "2"`](resolver.md#resolver-versions) must be | |
| set explicitly as they have no | |
| [`package.edition`][package-edition] to infer it from. | |
| [resolver version](resolver.md#resolver-versions). | |
| - Commands run in the a virtual workspace's directory will run against all workspace | |
| members by default, see [Package selection](#package-selection). |
There was a problem hiding this comment.
I'm not quite seeing the problem.
When I look at the previous paragraph, it ends on the note of the role of virtual workspaces contrasted with root packages
This is typically useful when there isn't a "primary" package, or
you want to keep all the packages organized in separate directories.
I tweaked the leading line for the new section a little bit which now reads
By having a workspace without a root package,
Could you help me understand what challenge you are seeing in the transition?
There was a problem hiding this comment.
Thanks for that. It feels better now. I guess it is because that there is an extra term “primary package”, so root package is a bit far when mentioned.
There was a problem hiding this comment.
There is still something missing. The doc never explains that a root package is effectively a default workspace member.
There was a problem hiding this comment.
This was implied in the old "Package selection" wording by saying that the package for the current working directory is used. However, I think the current wording is up for misinterpretation and I'm tweaking it in hopes that it will be less confusing.
Nothing else in the files does despite it all being optional and this will be clearer in a follow up when I specify the defaults.
This will make it easier to make future changes
The old statement that its a subset of `members` is incorrect because some members can be inferred and not show up in `members`. I tried to reword this to better convey the goal of what was being said
This also tweaks the wording to be clear that `default-members` always apply in the workspace root, not just for virtual workspaces.
For a user to read the reference and to understand when each type of workspace might be right for them, they have to know to also read the section on Package Selection. This reframes the section on needing to set `resolver = "2"` to being about differences when there isn't a root package and extends it to summarize a part of Package Selection, linking out to it. The hope is that this will make it all of the differences more discoverable without retreading too much of the same ground within Reference-style documentation. Part of rust-lang#13580
weihanglo
reviewed
Apr 25, 2024
weihanglo
approved these changes
Apr 25, 2024
|
@bors r+ Nice work! |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 26, 2024
Update cargo 9 commits in c9392675917adc2edab269eea27c222b5359c637..b60a1555155111e962018007a6d0ef85207db463 2024-04-23 19:35:19 +0000 to 2024-04-26 16:37:29 +0000 - fix(toml): Remove underscore field support in 2024 (rust-lang/cargo#13804) - fix: emit 1.77 syntax error only when msrv is incompatible (rust-lang/cargo#13808) - docs(ref): Index differences between virtual / real manifests (rust-lang/cargo#13794) - refactor(toml): extract dependency-to-source-id to function (rust-lang/cargo#13802) - Add where lint was set (rust-lang/cargo#13801) - fix(toml): Don't double-warn when underscore is used in workspace dep (rust-lang/cargo#13800) - fix(toml): Be more forceful with underscore/dash redundancy (rust-lang/cargo#13798) - Fix warning suppression for config.toml vs config compat symlinks (rust-lang/cargo#13793) - Cleanup linting system (rust-lang/cargo#13797) r? ghost
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 27, 2024
Update cargo 9 commits in c9392675917adc2edab269eea27c222b5359c637..b60a1555155111e962018007a6d0ef85207db463 2024-04-23 19:35:19 +0000 to 2024-04-26 16:37:29 +0000 - fix(toml): Remove underscore field support in 2024 (rust-lang/cargo#13804) - fix: emit 1.77 syntax error only when msrv is incompatible (rust-lang/cargo#13808) - docs(ref): Index differences between virtual / real manifests (rust-lang/cargo#13794) - refactor(toml): extract dependency-to-source-id to function (rust-lang/cargo#13802) - Add where lint was set (rust-lang/cargo#13801) - fix(toml): Don't double-warn when underscore is used in workspace dep (rust-lang/cargo#13800) - fix(toml): Be more forceful with underscore/dash redundancy (rust-lang/cargo#13798) - Fix warning suppression for config.toml vs config compat symlinks (rust-lang/cargo#13793) - Cleanup linting system (rust-lang/cargo#13797) r? ghost
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What does this PR try to resolve?
For a user to read the reference and to understand when each type of workspace might be right for them, they have to know to also read the section on Package Selection.
This reframes the section on needing to set
resolver = "2"to being about differences when there isn't a root package and extends it to summarize a part of Package Selection, linking out to it. The hope is that this will make it all of the differences more discoverable without retreading too much of the same ground within Reference-style documentation.Part of #13580
How should we test and review this PR?
Additional information
r? @weihanglo