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

Use same docs, for CI and deployment #12692

Closed
wants to merge 19 commits into from
Closed

Conversation

ameknite
Copy link
Contributor

@ameknite ameknite commented Mar 24, 2024

Objective

Solution

  • unify doc settings for CI and Deployment
    A new flag --deploy-docs was added.
    Now it build docs in nightly:
    CI: cargo r -p ci -- doc-check --nightly. Build docs with --workspace
    Deploy: cargo r -p ci -- doc-check --nightly --deploy-docs. Build docs with -p bevy

  • Fix some warnings to pass CI.

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-Build-System Related to build systems or continuous integration labels Mar 25, 2024
@james7132 james7132 requested a review from mockersf March 25, 2024 01:06
Copy link
Member

@BD103 BD103 left a comment

Choose a reason for hiding this comment

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

I have a few questions and a couple of suggestions. Sorry for the delay, I was away for the past few days. :)

@@ -1128,6 +1128,7 @@ wasm = true
[[example]]
name = "log_layers"
path = "examples/app/log_layers.rs"
doc-scrape-examples = true
Copy link
Member

Choose a reason for hiding this comment

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

Is this meant to be set to false? I believe the default is true, so this is redundant unless I'm missing something.

Docs, for future me's reference

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 was getting a warning to add the text.
Before adding doc-scrape-examples = true, the docs in those examples was not being checked by cargo doc

Copy link
Member

Choose a reason for hiding this comment

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

Odd, I wonder why those examples were getting skipped. Could you send a screenshot of the error message? Perhaps it will give further insight...

crates/bevy_winit/src/winit_config.rs Show resolved Hide resolved
examples/app/log_layers_ecs.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
examples/app/log_layers_ecs.rs Outdated Show resolved Hide resolved
examples/app/log_layers_ecs.rs Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved

test_suite.insert(
Check::DOC_CHECK,
vec![CITest {
command: cmd!(sh, "cargo doc {args...}"),
command: cmd!(sh, "cargo +nightly doc {args...}"),
Copy link
Member

@mockersf mockersf Mar 31, 2024

Choose a reason for hiding this comment

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

it's a pain to require nightly in the tools. if it's launched using stable, it should keep on stable and not stealthily use a nightly

it's not the first place it's used in this script, I would rather that this would be fixed rather than add more places where this appear

Copy link
Contributor Author

@ameknite ameknite Mar 31, 2024

Choose a reason for hiding this comment

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

Nightly is needed if you want to check correctly the docs as in docs.rs
I added a NIGHTLY flag so the user can decide how to use it

@alice-i-cecile
Copy link
Member

@Brezak could I get your review here?

Comment on lines +286 to +288
- uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ env.NIGHTLY_TOOLCHAIN }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment explaining why we need nightly. Something like:

Suggested change
- uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ env.NIGHTLY_TOOLCHAIN }}
# Use nightly so we can use `rustdoc-scrape-examples`. See rust-lang\rust#88791
- uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ env.NIGHTLY_TOOLCHAIN }}

@BD103 BD103 mentioned this pull request Apr 9, 2024
github-merge-queue bot pushed a commit that referenced this pull request Apr 9, 2024
# Objective

- Fixes #12905.

## Solution

- Use proper code `` tags for `TaskPoolBuilder::thread_name`.
- Remove leftover documentation in `TaskPool` referencing the deleted
`TaskPoolInner` struct.
- It may be possible to rephrase this, but I do not know enough about
the task pool to write something. (cc @james7132 who made the change
removing `TaskPoolInner`.)
- Ignore a buggy rustdoc lint that thinks `App` is already in scope for
`UpdateMode` doc. (Extracted from #12692.)
github-merge-queue bot pushed a commit that referenced this pull request Apr 13, 2024
# Objective

- I daily drive nightly Rust when developing Bevy, so I notice when new
warnings are raised by `cargo check` and Clippy.
- `cargo +nightly clippy` raises a few of these new warnings.

## Solution

- Fix most warnings from `cargo +nightly clippy`
- I skipped the docs-related warnings because some were covered by
#12692.
- Use `Clone::clone_from` in applicable scenarios, which can sometimes
avoid an extra allocation.
- Implement `Default` for structs that have a `pub const fn new() ->
Self` method.
- Fix an occurrence where generic constraints were defined in both `<C:
Trait>` and `where C: Trait`.
  - Removed generic constraints that were implied by the `Bundle` trait.

---

## Changelog

- `BatchingStrategy`, `NonGenericTypeCell`, and `GenericTypeCell` now
implement `Default`.
@ameknite ameknite mentioned this pull request Apr 14, 2024
github-merge-queue bot pushed a commit that referenced this pull request Apr 14, 2024
# Objective

- Fix some doc warnings 
- Add doc-scrape-examples to all examples

Moved from #12692 

I run `cargo +nightly doc --workspace --all-features --no-deps
-Zunstable-options -Zrustdoc-scrape-examples`

<details>

```
warning: public documentation for `GzAssetLoaderError` links to private item `GzAssetLoader`
  --> examples/asset/asset_decompression.rs:24:47
   |
24 | /// Possible errors that can be produced by [`GzAssetLoader`]
   |                                               ^^^^^^^^^^^^^ this item is private
   |
   = note: this link will resolve properly if you pass `--document-private-items`
   = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default

warning: `bevy` (example "asset_decompression") generated 1 warning
warning: unresolved link to `shape::Quad`
 --> examples/2d/mesh2d.rs:3:15
  |
3 | //! [`Quad`]: shape::Quad
  |               ^^^^^^^^^^^ no item named `shape` in scope
  |
  = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

warning: `bevy` (example "mesh2d") generated 1 warning
warning: unresolved link to `WorldQuery`
 --> examples/ecs/custom_query_param.rs:1:49
  |
1 | //! This example illustrates the usage of the [`WorldQuery`] derive macro, which allows
  |                                                 ^^^^^^^^^^ no item named `WorldQuery` in scope
  |
  = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
  = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

warning: `bevy` (example "custom_query_param") generated 1 warning
warning: unresolved link to `shape::Quad`
 --> examples/2d/mesh2d_vertex_color_texture.rs:4:15
  |
4 | //! [`Quad`]: shape::Quad
  |               ^^^^^^^^^^^ no item named `shape` in scope
  |
  = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

warning: `bevy` (example "mesh2d_vertex_color_texture") generated 1 warning
warning: public documentation for `TextPlugin` links to private item `CoolText`
  --> examples/asset/processing/asset_processing.rs:48:9
   |
48 | /// * [`CoolText`]: a custom RON text format that supports dependencies and embedded dependencies
   |         ^^^^^^^^ this item is private
   |
   = note: this link will resolve properly if you pass `--document-private-items`
   = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default

warning: public documentation for `TextPlugin` links to private item `Text`
  --> examples/asset/processing/asset_processing.rs:49:9
   |
49 | /// * [`Text`]: a "normal" plain text file
   |         ^^^^ this item is private
   |
   = note: this link will resolve properly if you pass `--document-private-items`

warning: public documentation for `TextPlugin` links to private item `CoolText`
  --> examples/asset/processing/asset_processing.rs:51:57
   |
51 | /// It also defines an asset processor that will load [`CoolText`], resolve embedded dependenc...
   |                                                         ^^^^^^^^ this item is private
   |
   = note: this link will resolve properly if you pass `--document-private-items`

warning: `bevy` (example "asset_processing") generated 3 warnings
warning: public documentation for `CustomAssetLoaderError` links to private item `CustomAssetLoader`
  --> examples/asset/custom_asset.rs:20:47
   |
20 | /// Possible errors that can be produced by [`CustomAssetLoader`]
   |                                               ^^^^^^^^^^^^^^^^^ this item is private
   |
   = note: this link will resolve properly if you pass `--document-private-items`
   = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default

warning: public documentation for `BlobAssetLoaderError` links to private item `CustomAssetLoader`
  --> examples/asset/custom_asset.rs:61:47
   |
61 | /// Possible errors that can be produced by [`CustomAssetLoader`]
   |                                               ^^^^^^^^^^^^^^^^^ this item is private
   |
   = note: this link will resolve properly if you pass `--document-private-items`
```

```
warning: `bevy` (example "mesh2d") generated 1 warning
warning: public documentation for `log_layers_ecs` links to private item `update_subscriber`
 --> examples/app/log_layers_ecs.rs:6:18
  |
6 | //! Inside the [`update_subscriber`] function we will create a [`mpsc::Sender`] and a [`mpsc::R...
  |                  ^^^^^^^^^^^^^^^^^ this item is private
  |
  = note: this link will resolve properly if you pass `--document-private-items`
  = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default

warning: unresolved link to `AdvancedLayer`
 --> examples/app/log_layers_ecs.rs:7:72
  |
7 | ... will go into the [`AdvancedLayer`] and the [`Receiver`](mpsc::Receiver) will
  |                        ^^^^^^^^^^^^^ no item named `AdvancedLayer` in scope
  |
  = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
  = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

warning: unresolved link to `LogEvents`
 --> examples/app/log_layers_ecs.rs:8:42
  |
8 | //! go into a non-send resource called [`LogEvents`] (It has to be non-send because [`Receiver`...
  |                                          ^^^^^^^^^ no item named `LogEvents` in scope
  |
  = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

warning: public documentation for `log_layers_ecs` links to private item `transfer_log_events`
 --> examples/app/log_layers_ecs.rs:9:30
  |
9 | //! From there we will use [`transfer_log_events`] to transfer log events from [`LogEvents`] to...
  |                              ^^^^^^^^^^^^^^^^^^^ this item is private
  |
  = note: this link will resolve properly if you pass `--document-private-items`

warning: unresolved link to `LogEvents`
 --> examples/app/log_layers_ecs.rs:9:82
  |
9 | ...nsfer log events from [`LogEvents`] to an ECS event called [`LogEvent`].
  |                            ^^^^^^^^^ no item named `LogEvents` in scope
  |
  = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

warning: public documentation for `log_layers_ecs` links to private item `LogEvent`
 --> examples/app/log_layers_ecs.rs:9:119
  |
9 | ...nts`] to an ECS event called [`LogEvent`].
  |                                   ^^^^^^^^ this item is private
  |
  = note: this link will resolve properly if you pass `--document-private-items`

warning: public documentation for `log_layers_ecs` links to private item `LogEvent`
  --> examples/app/log_layers_ecs.rs:11:49
   |
11 | //! Finally, after all that we can access the [`LogEvent`] event from our systems and use it.
   |                                                 ^^^^^^^^ this item is private
   |
   = note: this link will resolve properly if you pass `--document-private-items`
```

<details/>
@ameknite ameknite closed this May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build-System Related to build systems or continuous integration C-Docs An addition or correction to our documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider validating that docs can build as on docs.rs in CI
5 participants