-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Link to documentation-specific guidelines. #77195
Conversation
Changed because it's not obvious how to get from the previously used URL to the documentation-specific content. This is partly because the original URL was previously changed to point to different content: * https://github.com/rust-lang/rust/pull/74037/files#diff-242481015141f373dcb178e93cffa850L88 * rust-lang@3f6928f#diff-6a3371457528722a734f3c51d9238c13L12
r? @kennytm (rust_highfive has picked a reviewer for you, use r? to override) |
@@ -85,7 +85,7 @@ | |||
//! # Contributing changes to the documentation | |||
//! | |||
//! Check out the rust contribution guidelines [here]( | |||
//! https://rustc-dev-guide.rust-lang.org/getting-started.html). | |||
//! https://rustc-dev-guide.rust-lang.org/contributing.html#writing-documentation). | |||
//! The source for this documentation can be found on | |||
//! [GitHub](https://github.com/rust-lang/rust). |
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 doesn't seem specific, should we change this 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.
Are you referring to the [GitHub](https://github.com/rust-lang/rust)
link?
If so, yeah, I was working toward suggesting a PR for that too but it was a bit less "obvious" fix so was limiting this PR to just the one change.
The main question/issue I see is that the "Writing Documentation" section includes this sentence:
The source of doc.rust-lang.org is located in
src/doc
in the tree, and standard API documentation is generated from the source code itself.
So, depending on what "this documentation" is intended to refer to it's unclear whether the best approach is:
- Link to
library/std/src/lib.rs
- Remove "The source for this documentation..." sentence entirely.
- Replace "The source for this documentation..." sentence with "The source of doc.rust-lang.org is located in
src/doc
in the tree, and standard API documentation is generated from the source code itself (e.g.lib.rs
)." (And/or make the link additions to https://rustc-dev-guide.rust-lang.org/contributing.html.)
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 you could just include another suggestion here along with the patch. But I think linking to library/std/src/lib.rs
isn't good enough since people may want to edit other files.
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've put a PR in the dev guide repo with option 3: rust-lang/rustc-dev-guide#898
If that PR gets merged then perhaps we can decide if there's an issue with duplicating the content here?
The example link to library/std/src/lib.rs
was (a) the literal source for the documentation in question :) and (b) just intended as an example to help people start looking in the right place. Open to other suggestions of where to link instead.
Adding multiple commits to a PR via the GitHub interface (as I'm currently doing for such "drive-by" contributions) is complicated due to the inability to--if I recall correctly--squash commits as required by Rust? (Or maybe that's a per-repo configuration setting?) So, if it's possible to just fix the one issue in this PR, that would be preferable on my part. :)
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.
Adding multiple commits to a PR via the GitHub interface (as I'm currently doing for such "drive-by" contributions) is complicated due to the inability to--if I recall correctly--squash commits as required by Rust?
See also rust-lang/triagebot#821
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.
So, if it's possible to just fix the one issue in this PR, that would be preferable on my part
I think it makes sense to have the changes in one place so it's easier to find all the discussion. I won't block on squashing the commits :)
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.
Approach A
Based on the now merged #898, one possible approach is something like:
//! [GitHub](https://github.com/rust-lang/rust). | |
//! [GitHub](https://github.com/rust-lang/rust). | |
//! Standard API documentation is generated from | |
//! the source code itself (e.g. [`lib.rs`]) and the | |
//! source of `doc.rust-lang.org` is located in [`src/doc`] in the tree. | |
//! | |
//! [`lib.rs`]: https://github.com/rust-lang/rust/blob/master/library/std/src/lib.rs#L1 | |
//! [`src/doc`]: https://github.com/rust-lang/rust/tree/master/src/doc |
But if this file is being viewed via https://doc.rust-lang.org/std/index.html#contributing-changes-to-the-documentation then that wording seems potentially confusing?
Approach B
An alternate wording:
This document and other Standard API documentation is generated from the source code itself (e.g.
lib.rs
) which can be found inlibrary/
on GitHub.Source for other documentation hosted on
doc.rust-lang.org
is located insrc/doc
.
(Given the content of the latter paragraph is now already mentioned elsewhere, I'm not sure that it's necessarily justified to mention it here.)
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.
But if this file is being viewed via https://doc.rust-lang.org/std/index.html#contributing-changes-to-the-documentation then that wording seems potentially confusing?
Hmm, that doesn't seem too confusing to me: It says the documentation for the standard library lives in the source code for the standard library. I think lib.rs
is a little confusing though, maybe say library/std
instead.
Both of your approaches seem fine to me, I think both are more clear than the current links.
As a side observation, it seems like the PRs related to these issues are a contributing factor to the underlying issue of what needs to be mentioned/linked where: |
@bors r+ rollup Your current changes are good and I don't want to block them on doing a larger refactor. Please mention this PR in any follow-ups, though :) |
📌 Commit e4943ac has been approved by |
Link to documentation-specific guidelines. Changed contribution information URL because it's not obvious how to get from the current URL to the documentation-specific content. The current URL points to this "Getting Started" page, which contains nothing specific about documentation[*] and instead launches into how to *build* `rustc` which is not a strict prerequisite for contributing documentation fixes: * https://rustc-dev-guide.rust-lang.org/getting-started.html [*] The most specific content is a "Writing documentation" bullet point which is not itself a link to anything (I guess a patch for that might be helpful too). ### Why? Making this change will make it easier for people who wish to make small "drive by" documentation fixes (and read contribution guidelines ;) ) which I find are often how I start contributing to a project. (Exhibit A: rust-lang#77050 :) ) ### Background My impression is the change of content linked is an unintentional change due to a couple of other changes: * Originally, the link pointed to `contributing.md` which started with a "table of contents" linking to each section. But the content in `contributing.md` was removed and replaced with a link to the "Getting Started" section here: * rust-lang@3f6928f#diff-6a3371457528722a734f3c51d9238c13L1 But the changed link doesn't actually point to the equivalent content, which is now located here: * https://rustc-dev-guide.rust-lang.org/contributing.html (If the "Guide to Rustc Development" is now considered the canonical location of "How to Contribute" content it might be a good idea to merge some of the "Contributing" Introduction section into the "Getting Started" section.) * This was then compounded by changing the link from `contributing.md` to `contributing.html` here: * https://github.com/rust-lang/rust/pull/74037/files#diff-242481015141f373dcb178e93cffa850L88 In order to even find the new location of the previous `contributing.md` content I ended up needing to do a GitHub search of the `rust-lang` org for the phrase "Documentation improvements are very welcome". :D
Rollup of 10 pull requests Successful merges: - rust-lang#77195 (Link to documentation-specific guidelines.) - rust-lang#77629 (Cleanup of `eat_while()` in lexer) - rust-lang#77709 (Link Vec leak doc to Box) - rust-lang#77738 (fix __rust_alloc_error_handler comment) - rust-lang#77748 (Dead code cleanup in windows-gnu std) - rust-lang#77754 (Add TraitDef::find_map_relevant_impl) - rust-lang#77766 (Clarify the debug-related values should take boolean) - rust-lang#77777 (doc: disambiguate stat in MetadataExt::as_raw_stat) - rust-lang#77782 (Fix typo in error code description) - rust-lang#77787 (Update `changelog-seen` in config.toml.example) Failed merges: r? `@ghost`
@jyn514 Thanks! Yeah, real life caught up with me so I haven't gotten back to this. :) |
Changed contribution information URL because it's not obvious how to get from the current URL to the documentation-specific content.
The current URL points to this "Getting Started" page, which contains nothing specific about documentation[*] and instead launches into how to build
rustc
which is not a strict prerequisite for contributing documentation fixes:[*] The most specific content is a "Writing documentation" bullet point which is not itself a link to anything (I guess a patch for that might be helpful too).
Why?
Making this change will make it easier for people who wish to make small "drive by" documentation fixes (and read contribution guidelines ;) ) which I find are often how I start contributing to a project. (Exhibit A: #77050 :) )
Background
My impression is the change of content linked is an unintentional change due to a couple of other changes:
Originally, the link pointed to
contributing.md
which started with a "table of contents" linking to each section. But the content incontributing.md
was removed and replaced with a link to the "Getting Started" section here:But the changed link doesn't actually point to the equivalent content, which is now located here:
(If the "Guide to Rustc Development" is now considered the canonical location of "How to Contribute" content it might be a good idea to merge some of the "Contributing" Introduction section into the "Getting Started" section.)
This was then compounded by changing the link from
contributing.md
tocontributing.html
here:In order to even find the new location of the previous
contributing.md
content I ended up needing to do a GitHub search of therust-lang
org for the phrase "Documentation improvements are very welcome". :D