-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
75521 rustdoc book improvements #75778
75521 rustdoc book improvements #75778
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
@@ -1,14 +1,85 @@ | |||
# How to write documentation | |||
|
|||
Good documentation is not natural. There are opposing forces that make good |
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.
I was wondering: do we have to keep two whitespaces between sentences? cc @rust-lang/docs
|
||
## Configuring rustdoc | ||
|
||
There's two problems with this: first, why does it |
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.
There's two problems with this: first, why does it | |
There are two problems with this: first, why does it |
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.
I agree, I'll re-read it looking for more of these.
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.
I thought it looked more natural as a contraction, but either way is fine as long as it's consistent.
This is a great start! I think if we should also talk more about the arguments that allows to customize the HTML output (for example, adding themes or adding your own JS/CSS files, etc...). We can do that in another PR though (but if so, don't mark this PR as a fix for #75521 ;) ). |
Oh also: I'll definitely need a native english person to double-check the typos and other things. :) |
I agree with the themes included in this PR. Should I adjust the title to include work in process? |
Well, it remains "rustdoc improvements", so it's fine. But you might want to expand the description of the PR a bit if you do it in this PR. ;) |
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.
initial review tidbits, just felt it reads better, can ignore if you wish :D
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.
I may have gone a little overboard.
d7c21eb
to
0f605b0
Compare
The first line of description will be reused to describe the component in | ||
searches and module overviews. For example, the function `std::env::args()` | ||
above will be shown on the [`std::env`] module documentation. Multi-line | ||
summaries are also possible, however, concise writing is a goal of good | ||
documentation. |
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.
This is still not correct - it implies that only the first line of a multi-line summary will be shown. It should say that everything before a line break will be shown, but it's good practice to only have one line in the summary.
@@ -7,22 +7,22 @@ CSS, and JavaScript. | |||
|
|||
## Basic usage | |||
|
|||
Let's give it a try! Let's create a new project with Cargo: | |||
Let us give it a try! Create a new project with Cargo: |
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.
This also seems really stilted.
☔ The latest upstream changes (presumably #75740) made this pull request unmergeable. Please resolve the merge conflicts. Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:
|
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.
This mostly looks good to me - there's a lot of new information, thank you so much! I'm still a little unsure about expanding out all the contractions (especially since some have been missed, and it's now a little inconsistent).
Also this adds a link to the rustdoc book when you pass --help
, I'm not sure how I feel about that (#76427 (comment)). Maybe remove it for now and add it to #76427, so this PR won't be blocked on it?
Thanks for the feedback. Looks like there was an accidental merging of 2 tasks, thanks for identifying that. I'm a bit swamped now, but maybe this weekend I'll put some time into this. |
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.
I think any more changes can be done as follow-ups, this already looks really good. If you split out the change linking to the rustdoc book I think this ready to go.
It just requires to rebase to fix the merge conflict and then it's good to go! |
I will get this done. We moved and that's been taking all my time lately. |
No worries! Thanks for writing this up, it's super helpful :) If you find you don't have time to finish it up I can rebase it myself, too. |
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.
Normally we use a rebase-only workflow for PRs: https://rustc-dev-guide.rust-lang.org/git.html#no-merge-policy. Do you mind rebasing this instead to get rid of the merge conflict? There are instructions in https://rustc-dev-guide.rust-lang.org/git.html#rebasing, if you have trouble let me know and I'm happy to help.
I will, this was a 2 step process, making sure my master merge didn’t bring in extra artifacts. I was nervous I might lose my history. Thank you for the instructions!
|
Co-authored-by: Joshua Nelson <joshua@yottadb.com>
f2c82d7
to
881820a
Compare
@jyn514 we good now? |
Looks great, thanks! @bors r+ |
📌 Commit 881820a has been approved by |
wohoo! |
☀️ Test successful - checks-actions |
Added some guidelines about documenting with rustdoc
Fixes #75521