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

[Merged by Bors] - Doc warnings #2577

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ impl App {
/// A resource in Bevy represents globally unique data. Resources must be added to Bevy Apps
/// before using them. This happens with [`App::insert_resource`].
///
/// See also `init_resource` for resources that implement `Default` or [`FromResources`].
/// See also `init_resource` for resources that implement `Default` or [`FromWorld`].
///
/// ## Example
/// ```
Expand Down Expand Up @@ -390,7 +390,7 @@ impl App {

/// Initialize a resource in the current [App], if it does not exist yet
///
/// Adds a resource that implements `Default` or [`FromResources`] trait.
/// Adds a resource that implements `Default` or [`FromWorld`] trait.
/// If the resource already exists, `init_resource` does nothing.
///
/// ## Example
Expand Down Expand Up @@ -440,8 +440,9 @@ impl App {

/// Sets the main runner loop function for this Bevy App
///
/// 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)),
Copy link
Member

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

Copy link
Contributor Author

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.

Copy link
Member

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 😃

Copy link
Contributor Author

@Hoidigan Hoidigan Aug 9, 2021

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

Copy link

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.

Copy link
Member

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

/// [`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?

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 think that sounds good, I'll go through and do that.

/// but in some cases one might wish to implement their own main loop.
///
/// This method sets the main loop function, overwriting a previous runner if any.
///
Expand Down Expand Up @@ -491,7 +492,9 @@ impl App {
/// Bevy plugins can be grouped into a set of plugins. Bevy provides
/// built-in PluginGroups that provide core engine functionality.
///
/// The plugin groups available by default are [`DefaultPlugins`] and [`MinimalPlugins`].
/// The plugin groups available by default are
/// [`DefaultPlugins`](https://docs.rs/bevy_internal/0.5.0/bevy_internal/struct.DefaultPlugins.html)
/// and [`MinimalPlugins`](https://docs.rs/bevy_internal/0.5.0/bevy_internal/struct.MinimalPlugins.html).
///
/// ## Example
/// ```
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_asset/src/asset_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,8 @@ impl AssetServer {
/// The absolute Path to the asset is "ROOT/ASSET_FOLDER_NAME/path".
///
/// By default the ROOT is the directory of the Application, but this can be overridden by
/// setting the `"CARGO_MANIFEST_DIR"` environment variable (see https://doc.rust-lang.org/cargo/reference/environment-variables.html)
/// setting the `"CARGO_MANIFEST_DIR"` environment variable
/// (see <https://doc.rust-lang.org/cargo/reference/environment-variables.html>)
/// to another directory. When the application is run through Cargo, then
/// `"CARGO_MANIFEST_DIR"` is automatically set to the root folder of your crate (workspace).
///
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ use std::{any::TypeId, collections::HashMap};
/// ```
///
/// # Safety
/// [Bundle::component_id] must return the ComponentId for each component type in the bundle, in the
/// [Bundle::component_ids] must return the ComponentId for each component type in the bundle, in the
/// _exact_ order that [Bundle::get_components] is called.
/// [Bundle::from_components] must call `func` exactly once for each [ComponentId] returned by
/// [Bundle::component_id]
/// [Bundle::component_ids]
pub unsafe trait Bundle: Send + Sync + 'static {
/// Gets this [Bundle]'s component ids, in the order of this bundle's Components
fn component_ids(components: &mut Components) -> Vec<ComponentId>;
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/change_detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ impl_debug!(ResMut<'a, T>, Component);

/// Unique borrow of a non-[`Send`] resource.
///
/// Only [`Send`] resources may be accessed with the [`ResMut`] [`SystemParam`]. In case that the
/// Only [`Send`] resources may be accessed with the [`ResMut`] [`crate::system::SystemParam`]. In case that the
/// resource does not implement `Send`, this `SystemParam` wrapper can be used. This will instruct
/// the scheduler to instead run the system on the main thread so that it doesn't send the resource
/// over to another thread.
Expand Down
7 changes: 4 additions & 3 deletions crates/bevy_pbr/src/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use bevy_reflect::TypeUuid;
use bevy_render::{color::Color, renderer::RenderResources, shader::ShaderDefs, texture::Texture};

/// A material with "standard" properties used in PBR lighting
/// Standard property values with pictures here https://google.github.io/filament/Material%20Properties.pdf
/// Standard property values with pictures here <https://google.github.io/filament/Material%20Properties.pdf>
#[derive(Debug, RenderResources, ShaderDefs, TypeUuid)]
#[uuid = "dace545e-4bc6-4595-a79d-c224fc694975"]
pub struct StandardMaterial {
Expand Down Expand Up @@ -50,7 +50,7 @@ impl Default for StandardMaterial {
base_color: Color::rgb(1.0, 1.0, 1.0),
base_color_texture: None,
// This is the minimum the roughness is clamped to in shader code
// See https://google.github.io/filament/Filament.html#materialsystem/parameterization/
// See <https://google.github.io/filament/Filament.html#materialsystem/parameterization/>
// It's the minimum floating point value that won't be rounded down to 0 in the
// calculations used. Although technically for 32-bit floats, 0.045 could be
// used.
Expand All @@ -59,7 +59,8 @@ impl Default for StandardMaterial {
// This is just a default for mostly-dielectric
metallic: 0.01,
// Minimum real-world reflectance is 2%, most materials between 2-5%
// Expressed in a linear scale and equivalent to 4% reflectance see https://google.github.io/filament/Material%20Properties.pdf
// Expressed in a linear scale and equivalent to 4% reflectance see
/// <https://google.github.io/filament/Material%20Properties.pdf>
metallic_roughness_texture: None,
reflectance: 0.5,
normal_map: None,
Expand Down