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(contrib): Move overview to lib #11809

Merged
merged 3 commits into from
Mar 9, 2023
Merged

docs(contrib): Move overview to lib #11809

merged 3 commits into from
Mar 9, 2023

Conversation

epage
Copy link
Contributor

@epage epage commented Mar 7, 2023

What does this PR try to resolve?

Goals

  • Move docs closer to their use to make them more likely to be updated
  • More exhaustively rely on intra-doc links to keep the links valid through refactorings

To do this, I am exploring moving contributing documentation to their relevant libraries and linking out to them instead.

How should we test and review this PR?

Look at the PR a commit at a time

cargo doc --open
cargo doc --allow-private-items --open

Additional information

In moving the content over, I felt a lot of our current intro documentation was a bit verbose, so I tried to shrink it down so that adding more content would not make it overwhelming

Unfortunately, the intra-doc links seem to follow the same visibility
rule as using the API items, meaning that for lib.rs to link to a
mod, it has to have visibility of the mod, requiring some mods to be
made pub(crate) that previously weren't. I've gone ahead and done
this for now as the goal of this effort is for us to catch broken docs
through refactorings by using intra-doc links.

For now, the contrib docs page that was moved just links to the nightly
docs. Over time, all of this will just be collapsed into the
architecture page which will link out to the nightly docs.

This opens more room for additional content
Unfortunately, the intra-doc links seem to follow the same visibility
rule as using the API items, meaning that for `lib.rs` to link to a
`mod`, it has to have visibility of the `mod`, requiring some mods to be
made `pub(crate)` that previously weren't.  I've gone ahead and done
this for now as the goal of this effort is for us to catch broken docs
through refactorings by using intra-doc links.

For now, the contrib docs page that was moved just links to the nightly
docs.  Over time, all of this will just be collapsed into the
architecture page which will link out to the nightly docs.
@rustbot
Copy link
Collaborator

rustbot commented Mar 7, 2023

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-build-execution Area: anything dealing with executing the compiler A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 7, 2023
@epage
Copy link
Contributor Author

epage commented Mar 7, 2023

How do we feel about exposing mods as pub(crate) to allow intra-doc links?

Any other feedback on this before I commit more time to this effort?

@jyn514
Copy link
Member

jyn514 commented Mar 7, 2023

cc rust-lang/rust#83049

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.

Looks way better!

Approved as I feel great!

src/doc/contrib/src/architecture/codebase.md Show resolved Hide resolved
@@ -33,14 +33,14 @@

pub mod artifact;
mod build_config;
mod build_context;
pub(crate) mod build_context;
Copy link
Member

Choose a reason for hiding this comment

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

Not really a big deal. Just a bit worried making these modules public. Maintainer now need to gatekeep any misuse during the review.

src/cargo/lib.rs Outdated Show resolved Hide resolved
//! - [`credential`](https://github.com/rust-lang/cargo/tree/master/crates/credential)
//! This subdirectory contains several packages for implementing the
//! experimental
//! [credential-process](https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#credential-process)
Copy link
Member

Choose a reason for hiding this comment

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

The layout looks fine here, though I prefer reference-style links. They are more readable at source code level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I try them every once in a while but always feel like they are more of a pain to initially write (duplicating the referencing, jumping back and forth) and then dealing with later (more places to touch when updating).

src/cargo/lib.rs Outdated Show resolved Hide resolved
src/cargo/lib.rs Outdated Show resolved Hide resolved
@epage epage marked this pull request as ready for review March 9, 2023 16:46
@weihanglo
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Mar 9, 2023

📌 Commit 0f9bfb9 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 Mar 9, 2023
@bors
Copy link
Contributor

bors commented Mar 9, 2023

⌛ Testing commit 0f9bfb9 with merge e1605eb...

@bors
Copy link
Contributor

bors commented Mar 9, 2023

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

@bors bors merged commit e1605eb into rust-lang:master Mar 9, 2023
@epage epage deleted the contrib branch March 13, 2023 18:49
bors added a commit that referenced this pull request Mar 13, 2023
docs(contrib): Point compilation docs to doc comments

This is a follow up to #11809, merging the description of compilation with what is in the source code, leaving a breadcrumb for people who were used to going to the old page (for now).  The new entry point for finding this is the doc comment in `lib.rs`

Like with #11809, this also meant increasing the visibility of a mod.  This mod is mostly re-exported already, so this doesn't seem too bad.  I pointed directly to the `job _queue` mod rather than `JobQueue` because the mod had more of an architecture discussion.  For `drain_the_queue`, I also pointed at the mod because it talks about it and this avoided making `DrainState` and `drain_the_queue` `pub(crate)`.

This still leaves
- Files
  - Part of this indexes the architecture based on files generated and should be in `lib.rs`
  - Part of this is filesystem best practices and should be moved out of the architecture overview into some kind of Implementation Practices
- Package and Resolution
- Console Output.  This also likely belongs in an Implementation section
- Likely stuff in the testing section
epage added a commit to epage/cargo that referenced this pull request Mar 14, 2023
This is a follow up to rust-lang#11809

The rest of the file docs will need to be handled in a different way as
they are details for people implementing file system interactions.
epage added a commit to epage/cargo that referenced this pull request Mar 14, 2023
This is a follow up to rust-lang#11809.

On top of what was in the old contrib, I added links out for
`.cargo/config.toml`.  I'm assuming there are more files that would be
worth indexing here but we can add those over time.

My focus was on linking to the high-detail documentation.  In some
cases, this meant increasing visibility to make rustdoc happy.  In the
registry case, `sources::registry` is a great document to link to so I
instead dropped links that rustdoc couldn't handle as they were to
details covered in the bigger document or can easily be derived from it.

The rest of the file docs will need to be handled in a different way as
they are details for people implementing file system interactions.
weihanglo added a commit to weihanglo/rust that referenced this pull request Mar 14, 2023
14 commits in 7d3033d2e59383fd76193daf9423c3d141972a7d..4a3c588b1f0a8e2dc8dd8789dbf3b6a71b02ed49
2023-03-08 17:05:08 +0000 to 2023-03-14 14:05:36 +0000
- ci: make clean-test-output a script for reuse (rust-lang/cargo#11848)
- Accurately show status when downgrading dependencies (rust-lang/cargo#11839)
- docs(contrib): Move Design Principles earlier in the book (rust-lang/cargo#11842)
- docs(contrib): Point compilation docs to doc comments (rust-lang/cargo#11841)
- `cargo install --git` multiple packages with binaries found hint (rust-lang/cargo#11835)
- Disable flaky auth tests when `gitoxide` runs them (rust-lang/cargo#11830)
- Add some documentation on writing cross-compilation tests (rust-lang/cargo#11825)
- chore: Use sparse protocol on stable CI (rust-lang/cargo#11829)
- Notice for potential unexpected shell expansions in help text of `cargo-add` (rust-lang/cargo#11826)
- Add tracking issue to gitoxide unstable docs (rust-lang/cargo#11822)
- Bump crates-io to 0.36.0 (rust-lang/cargo#11820)
- Bump to 0.71.0; update changelog (rust-lang/cargo#11815)
- docs(contrib): Move overview to lib (rust-lang/cargo#11809)
- Fix semver check for 1.68 (rust-lang/cargo#11817)
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 15, 2023
Update cargo

14 commits in 7d3033d2e59383fd76193daf9423c3d141972a7d..4a3c588b1f0a8e2dc8dd8789dbf3b6a71b02ed49
2023-03-08 17:05:08 +0000 to 2023-03-14 14:05:36 +0000
- ci: make clean-test-output a script for reuse (rust-lang/cargo#11848)
- Accurately show status when downgrading dependencies (rust-lang/cargo#11839)
- docs(contrib): Move Design Principles earlier in the book (rust-lang/cargo#11842)
- docs(contrib): Point compilation docs to doc comments (rust-lang/cargo#11841)
- `cargo install --git` multiple packages with binaries found hint (rust-lang/cargo#11835)
- Disable flaky auth tests when `gitoxide` runs them (rust-lang/cargo#11830)
- Add some documentation on writing cross-compilation tests (rust-lang/cargo#11825)
- chore: Use sparse protocol on stable CI (rust-lang/cargo#11829)
- Notice for potential unexpected shell expansions in help text of `cargo-add` (rust-lang/cargo#11826)
- Add tracking issue to gitoxide unstable docs (rust-lang/cargo#11822)
- Bump crates-io to 0.36.0 (rust-lang/cargo#11820)
- Bump to 0.71.0; update changelog (rust-lang/cargo#11815)
- docs(contrib): Move overview to lib (rust-lang/cargo#11809)
- Fix semver check for 1.68 (rust-lang/cargo#11817)

r? `@ghost`
epage added a commit to epage/cargo that referenced this pull request Mar 15, 2023
This is a follow up to rust-lang#11809.

On top of what was in the old contrib, I added links out for
`.cargo/config.toml`.  I'm assuming there are more files that would be
worth indexing here but we can add those over time.

My focus was on linking to the high-detail documentation.  In some
cases, this meant increasing visibility to make rustdoc happy.  In the
registry case, `sources::registry` is a great document to link to so I
instead dropped links that rustdoc couldn't handle as they were to
details covered in the bigger document or can easily be derived from it.

The rest of the file docs will need to be handled in a different way as
they are details for people implementing file system interactions.
bors added a commit that referenced this pull request Mar 15, 2023
docs(contrib): Create a file overview in the nightly docs

This is a follow up to #11809.

On top of what was in the old contrib, I added links out for `.cargo/config.toml`.  I'm assuming there are more files that would be worth indexing here but we can add those over time.

My focus was on linking to the high-detail documentation.  In some cases, this meant increasing visibility to make rustdoc happy.  In the registry case, `sources::registry` is a great document to link to so I instead dropped links that rustdoc couldn't handle as they were to details covered in the bigger document or can easily be derived from it.

The rest of the file docs will need to be handled in a different way as they are details for people implementing file system interactions.
bors added a commit that referenced this pull request Mar 21, 2023
docs(contrib): Pull impl info out of architecture

This is a follow up to #11809.

Personally, I found mixing this stuff with architecture less than ideal as it buried the more practical information among details that might not have been as important.  With us moving architecture information into doc comments, this provides us an opportunity to rectify this.

Not a fan of the name of this chapter but its a start.

I've left in the old architecture chapter as there is still content to find a home for (resolver).
bors added a commit that referenced this pull request Mar 21, 2023
docs(contrib): Move higher level resolver docs into doc comments

This is a follow up to #11809.

I chose `ops::resolve` for most of the old documentation as this because the docs cover the
higher level details that include it, like `Cargo.lock` file, while
`core::resolver` is more of the algorithm.
@ehuss ehuss added this to the 1.70.0 milestone Mar 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-execution Area: anything dealing with executing the compiler A-documenting-cargo-itself Area: Cargo's documentation 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