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

revamp doc-build chapter #1402

Merged
merged 6 commits into from
Jul 31, 2022
Merged

revamp doc-build chapter #1402

merged 6 commits into from
Jul 31, 2022

Conversation

tshepang
Copy link
Member

@tshepang tshepang commented Jul 18, 2022

it's a tiny chapter, so made it lightweight by doing bullet points instead of section headers

@tshepang tshepang marked this pull request as ready for review July 19, 2022 20:42
src/building/compiler-documenting.md Outdated Show resolved Hide resolved
Comment on lines 3 to 4
You can run `rustdoc` directly on a file to make sure the HTML is correct,
which runs quickly.
Copy link
Member

Choose a reason for hiding this comment

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

This seems a little too concise to me. I'd like to talk that this section is to document things on rust-lang/rust like std or submodule docs at first.

Copy link
Member Author

Choose a reason for hiding this comment

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

should I remove this mention... I only kept it because I found it there, and I feel it's not worth a mention

Copy link
Member

@JohnTitor JohnTitor Jul 23, 2022

Choose a reason for hiding this comment

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

and I feel it's not worth a mention

Hmm, why? I think describing what this page talks about at first makes some sense as not all readers will read through the entire chapter.

Copy link
Member Author

Choose a reason for hiding this comment

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

I meant only the part about rustdoc being able to be run on a single file

Copy link
Member

Choose a reason for hiding this comment

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

I mean you removed the "You might want to build documentation of the various components available like the standard library." sentence but the introduction now sounds a little too concise.

Copy link
Member Author

Choose a reason for hiding this comment

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

my latest commit is also concise, but it explains what the chapter is about... whatyouthink

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @JohnTitor, "toolchain components" seems like jargon that won't be familiar to most readers. I think "the various components available, like the standard library or compiler," would be more clear.

tshepang and others added 2 commits July 23, 2022 00:16
Co-authored-by: Yuki Okushi <jtitor@2k36.org>
Comment on lines 23 to 24
the compiler and rustdoc get built to make sure everything is okay,
and then it documents the files.
Copy link
Member

Choose a reason for hiding this comment

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

hmm, this sentence is saying a lot of complicated things in very few words.

  • rustdoc doesn't check that programs are valid, so you have to run rustc directly to make sure the program compiles
  • rustdoc uses the built metadata to document the compiler (it's very unclear in this sentence whether "it" refers to rustdoc or bootstrap).

unless it's talking about stage1, in which case it's just completely wrong - the reason you have to build rustdoc is so you can run it, because it might be different from the beta rustdoc.

Copy link
Member

Choose a reason for hiding this comment

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

(this is also one of the few cases where --keep-stage 0 is sound - if you've only modified documentation, it can't affect the build artifacts, so it's ok to only rebuild the documentation and not rustdoc itself.)

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure how much detail to provide, but please have a look at the small change I made

src/building/compiler-documenting.md Outdated Show resolved Hide resolved
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

This looks a lot better than before, thanks :)

@@ -20,8 +20,8 @@ either in whole or individually.
```

First,
the compiler and rustdoc get built to make sure everything is okay,
and then it documents the files.
the compiler gets built to make sure rustdoc compiles,
Copy link
Member

Choose a reason for hiding this comment

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

this is super nitpicky and you can ignore it if you like, but I still feel like "make sure rustdoc compiles" is ambiguous. We're not checking that it compiles, we're actually compiling it. I would be less worried about this if it weren't for the fact that check is a cargo/x.py subcommand 😅

Maybe it makes more sense to just say "first rustdoc is built," ?

Copy link
Member Author

Choose a reason for hiding this comment

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

glad you are "nitpicky"... was not too happy with it, so thanks for this

tshepang and others added 2 commits July 31, 2022 07:09
Co-authored-by: Joshua Nelson <github@jyn.dev>
@tshepang tshepang merged commit 04f3cf0 into rust-lang:master Jul 31, 2022
@tshepang tshepang deleted the fixes branch July 31, 2022 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants