Skip to content
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(ref): Clarify workspace settings #11082

Merged
merged 9 commits into from
Sep 16, 2022
Merged

docs(ref): Clarify workspace settings #11082

merged 9 commits into from
Sep 16, 2022

Conversation

epage
Copy link
Contributor

@epage epage commented Sep 13, 2022

What does this PR try to resolve?

In reviewing the status of #10625, I was reminded

  • that we are having growing pains with the workspace documentation
  • that workspace.resolver isn't documented

So I re-organized the workspace docs to have a high level intro / behavior description and then to focus on being a field reference, much like manifest.md. I could see splitting it specifically into tutorial/reference like the overriding dependencies document does it.

When adding workspace.resolver, I remembered in the nested workspace discussion there were other workspace related sections that are not present. We now link out to profile, patch, and replace. In doing this, I realized that patch and replace do not specify their workspace behavior, so I do that.

How should we test and review this PR?

Look at it commit by commit to get more digestible chunks. Unfortunately, the first commit didn't split up so easily.

Additional information

Other information you want to mention in this PR, such as prior arts,
future extensions, an unresolved problem, or a TODO list.
-->

In looking over rust-lang#10625, I remembered we've been having growing pains
with the workspace documentation.  It was originally written when there
were a lot fewer workspace features.  As more workspace features have
been added, they've been tacked on to the documentation.

This re-thinks the documentation by focusing on the schema, much like
`manifest.md` does.
This is a part of the schema we were missing.  A first step before
encouraging people to use it is to document it!
In a workspace, only the workspace's profile is respected.  This is
already documented in the profile documentation but we should raise
visibility of it within the workspace documentation.
The workspace behavior doesn't seem to be documented at all, so a blurb
was brought in that is like the profile blurb.  The workspace docs then
link out to it so users can be aware of this special workspace behavior.
@rust-highfive
Copy link

r? @weihanglo

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 13, 2022
Copy link
Member

@Muscraft Muscraft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes make it much easier to read!

src/doc/src/reference/workspaces.md Outdated Show resolved Hide resolved
src/doc/src/reference/workspaces.md Outdated Show resolved Hide resolved
src/doc/src/reference/workspaces.md Show resolved Hide resolved
src/doc/src/reference/workspaces.md Outdated Show resolved Hide resolved
* All packages share a common `Cargo.lock` file which resides in the
*workspace root*.
* All packages share a common [output directory], which defaults to a
directory named `target` in the *workspace root*.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the benefits of sharing a Cargo.lock and one target dir be explained?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For output, let's rely on the information in that link. For cargo.lock, let's mirror output and provide a link for more details.

@weihanglo weihanglo added A-documenting-cargo-itself Area: Cargo's documentation A-workspaces Area: workspaces labels Sep 13, 2022
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This link needs an update as well.

[virtual workspace]: workspaces.md#virtual-manifest

src/doc/src/reference/workspaces.md Outdated Show resolved Hide resolved
src/doc/src/reference/workspaces.md Outdated Show resolved Hide resolved
@weihanglo
Copy link
Member

It seems good if we can ship this doc enhancement in 1.64. Should we send a PR to beta as well?

@epage epage force-pushed the workspace branch 2 times, most recently from 4411d28 to 4ea5dce Compare September 16, 2022 14:26
@weihanglo
Copy link
Member

Thank you!

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 16, 2022

📌 Commit f39f8dc has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 16, 2022
@bors
Copy link
Collaborator

bors commented Sep 16, 2022

⌛ Testing commit f39f8dc with merge 362ce33...

@bors
Copy link
Collaborator

bors commented Sep 16, 2022

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 362ce33 to master...

@bors bors merged commit 362ce33 into rust-lang:master Sep 16, 2022
@epage epage deleted the workspace branch September 16, 2022 16:33
@weihanglo weihanglo mentioned this pull request Sep 16, 2022
bors added a commit that referenced this pull request Sep 16, 2022
weihanglo added a commit to weihanglo/rust that referenced this pull request Sep 16, 2022
3 commits in 4bcb3c65e440a12044092b85ffea8fac6cb96f42..387270bc7f446d17869c7f208207c73231d6a252
2022-08-17 21:01:34 +0000 to 2022-09-16 20:18:27 +0000

- Beta backport rust-lang/cargo#11082 (rust-lang/cargo#11097)
- [Beta] Run `reach_max_unpack_size` test only on debug build (rust-lang/cargo#11090)
- [beta] Fix for CVE-2022-36113 and CVE-2022-36114 (rust-lang/cargo#11088)
weihanglo added a commit to weihanglo/rust that referenced this pull request Sep 18, 2022
8 commits in 082503982ea0fb7a8fd72210427d43a2e2128a63..73ba3f35e0205844418260722c11602113179c4a
2022-09-13 17:49:38 +0000 to 2022-09-18 06:38:16 +0000

- Revert "Clarify when cargo detects changes" (rust-lang/cargo#11107)
- Fix links to workspace inheritance headings in workspace docs (rust-lang/cargo#11103)
- docs(ref): Clarify workspace settings (rust-lang/cargo#11082)
- Update comment about ResolveVersion default version (rust-lang/cargo#11095)
- [master] Run `reach_max_unpack_size` test only on debug build (rust-lang/cargo#11091)
- Clarify when cargo detects changes (rust-lang/cargo#11092)
- [master] Fix for CVE-2022-36113 and CVE-2022-36114 (rust-lang/cargo#11089)
- Expose cargo add internals as edit API (rust-lang/cargo#11059)
bors pushed a commit to rust-lang-ci/rust that referenced this pull request Sep 19, 2022
3 commits in 4bcb3c65e440a12044092b85ffea8fac6cb96f42..387270bc7f446d17869c7f208207c73231d6a252
2022-08-17 21:01:34 +0000 to 2022-09-16 20:18:27 +0000

- Beta backport rust-lang/cargo#11082 (rust-lang/cargo#11097)
- [Beta] Run `reach_max_unpack_size` test only on debug build (rust-lang/cargo#11090)
- [beta] Fix for CVE-2022-36113 and CVE-2022-36114 (rust-lang/cargo#11088)
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 19, 2022
Update cargo (CVE fixes included)

8 commits in 082503982ea0fb7a8fd72210427d43a2e2128a63..73ba3f35e0205844418260722c11602113179c4a
2022-09-13 17:49:38 +0000 to 2022-09-18 06:38:16 +0000

- Revert "Clarify when cargo detects changes" (rust-lang/cargo#11107)
- Fix links to workspace inheritance headings in workspace docs (rust-lang/cargo#11103)
- docs(ref): Clarify workspace settings (rust-lang/cargo#11082)
- Update comment about ResolveVersion default version (rust-lang/cargo#11095)
- [master] Run `reach_max_unpack_size` test only on debug build (rust-lang/cargo#11091)
- Clarify when cargo detects changes (rust-lang/cargo#11092)
- [master] Fix for GHSA-rfj2-q3h3-hm5j and GHSA-2hvr-h6gw-qrxp (rust-lang/cargo#11089)
- Expose cargo add internals as edit API (rust-lang/cargo#11059)
@ehuss ehuss added this to the 1.65.0 milestone Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation A-workspaces Area: workspaces S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants