-
-
Notifications
You must be signed in to change notification settings - Fork 3.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
[Merged by Bors] - Doc warnings #2577
Conversation
Made the links clickable by surrounding with <>, which should stop the warnings of "use an automatic link instead"
This is by adding a link to the docs.rs pages for external structs.
Fix links that are (theoritically) broken due to typos and refactoring.
bors try |
tryBuild failed: |
bors try |
@cart Could I get the workflow run for this? I can't do it since I'm a first time contributor. |
bors try |
tryBuild failed: |
What should I do to get that to work? |
@Hoidigan, clicking through, I'm seeing
Looks like a networking error that isn't your fault. #2040 has more contributing advice for you though; it's just waiting on final review now <3 |
bors try |
crates/bevy_app/src/app.rs
Outdated
/// Usually the main loop is handled by Bevy integrated plugins ([`WinitPlugin`]), but | ||
/// in some cases one might wish to implement their own main loop. | ||
/// Usually the main loop is handled by Bevy integrated plugins | ||
/// ([`WinitPlugin`](https://docs.rs/bevy_winit/0.5.0/bevy_winit/struct.WinitPlugin.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.
full links to docs.rs should be avoided
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.
What should I change it to get the correct link? Replace the 0.5.0 with *?
We can't point it to WinitPlugin from inside that crate, because it doesn't have that as a dependency.
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.
you should be able to add a dev dependencies to bevy_winit
that will allow doc links to work 😃
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 don't think it does... It doesn't bring it into scope for the doc comments. I'm fairly certain we'd have to bring it in as a full dependency.
rust-lang/api-guidelines#186
Oh here we are, I was trying to dig and find this! https://discord.com/channels/691052431525675048/743559241461399582/871090893162156082
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.
Cargo doesn't provide dev-dependencies when documenting. @Nemo157 has suggested doc-dependencies
a few times but it doesn't currently exist.
See also rust-lang/rust#74481.
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.
Oh right, dev dependencies are available when testing doc code blocks but not when building docs...
The other place where we have a similar link is
bevy/crates/bevy_ecs/src/event.rs
Line 120 in 90586a4
/// [`App::add_event`]: https://docs.rs/bevy/*/bevy/app/struct.App.html#method.add_event |
If we consider than the main target for those links will be docs.rs users, I think it should link to the root crate (bevy
instead of bevy_winit
) and use *
for version. What do you think?
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 that sounds good, I'll go through and do that.
@Hoidigan could you run |
I should be able to this Sunday, sorry about the wait. I'm away and don't have my computer |
(Sorry I'm a bit slow) Running cargo fmt --all modifies all 408 files. I only committed the changes in the 3 files that the ci complained about, but that was little weird. I think it was newline issues, but I don't actually know. |
bors r+ |
Fixes #2576 Add <> to links where needed, fix typo and refactored names, and add docs.rs links for external items.
Fixes #2576 Add <> to links where needed, fix typo and refactored names, and add docs.rs links for external items.
Fixes #2576
Add <> to links where needed, fix typo and refactored names, and add docs.rs links for external items.