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

Tracking issue: deny missing docs, one crate at a time #3492

Open
16 of 45 tasks
alice-i-cecile opened this issue Dec 30, 2021 · 26 comments
Open
16 of 45 tasks

Tracking issue: deny missing docs, one crate at a time #3492

alice-i-cecile opened this issue Dec 30, 2021 · 26 comments
Labels
C-Docs An addition or correction to our documentation C-Tracking-Issue An issue that collects information about a broad development initiative D-Straightforward Simple bug fixes and API improvements, docs, test and examples

Comments

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Dec 30, 2021

Docs are great! However, it's easy to let them atrophy.

As Bevy grows and stabilizes, we want to be able to turn on #![warn(missing_docs)] (which will cause CI to fail if violated). This is helpful because it helpful for new users and contributors, and ensures that our code base's documentation state doesn't regress.

However, not everything is stable or well documented enough to do so already.
We should turn this on one crate at a time, once it hits 100% coverage. If there's a crate

Let's check the current doc coverage.

  1. Set the following environment variable (on Windows, I used the $Env: syntax): RUSTDOCFLAGS="-Z unstable-options --show-coverage"
  2. Run cargo +nightly doc --workspace --all-features --no-deps

Run on 2023-03-14:
Crate | Docs Coverage

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation D-Trivial Nice and easy! A great choice to get started with Bevy C-Tracking-Issue An issue that collects information about a broad development initiative labels Dec 30, 2021
@alice-i-cecile
Copy link
Member Author

So, the steps to tackle this are simple enough.

  1. Pick a crate that is small, or nearly documented.
  2. Make a PR to finish documenting the crate. If the crate is large, this needs to be split across several PRs to make reviewing easier.
  3. Once you've reached 100% coverage, add #!warn(missing_docs) to the start of that crate's lib.rs file.
  4. Repeat 20 or so times.

If you're looking for good candidates, I think the low-hanging fruit here is:

  • bevy_utils
  • bevy_tasks
  • bevy_math
  • bevy_app
  • bevy_dynamic_plugin
  • bevy_log
  • bevy_core
  • bevy_gilrs
  • bevy_transform
  • bevy_internal`

plus the crates that are already at 100% coverage. Feel free to ping me if you would like review or help understanding the code.

@mfdorst
Copy link
Contributor

mfdorst commented Dec 30, 2021

Does it make sense to add that to bevy_crevice since it's really on the crevice maintainers to keep their docs up to date?

@alice-i-cecile
Copy link
Member Author

Does it make sense to add that to bevy_crevice since it's really on the crevice maintainers to keep their docs up to date?

Yeah, I agree with you, since we're basically just mirroring it.

@ghost
Copy link

ghost commented Dec 30, 2021

I'm going to document the bevy_math crate and will link the PR here once I'm done.

@james7132
Copy link
Member

Mentioned this on Discord, but I'll take on bevy_tasks.

@ghost
Copy link

ghost commented Jan 1, 2022

I'm going to add the warning to bevy_dylib and update the documentation if needed.

@Sheepyhead
Copy link
Contributor

I'll be adding the warning to bevy_internal and adding missing docs

@Sheepyhead
Copy link
Contributor

I'll be looking at bevy_transform next

bors bot pushed a commit that referenced this issue Jan 1, 2022
…3514)

# Objective
This contributes documentation that should cover the entirety of bevy_internal as requested in #3492 

## Solution
warn(missing_docs) has been activated and documentation has been added to missing parts so that no warnings appear from this setting
@Sheepyhead
Copy link
Contributor

I'll take a look at bevy_ui but I make no promises, I think there are parts of this crate I cannot explain yet

@alice-i-cecile
Copy link
Member Author

I'll take a look at bevy_ui but I make no promises, I think there are parts of this crate I cannot explain yet

Sounds good, it's a big crate so even partial progress there will be very helpful.

bors bot pushed a commit that referenced this issue Jan 2, 2022
This PR is part of the issue #3492.

# Objective
- Add and update the bevy_dylib documentation to achieve a 100% documentation coverage.
- Add the #![warn(missing_docs)] lint to keep the documentation coverage for the future.

# Solution
- Add and update the bevy_dylib documentation.
- Add the #![warn(missing_docs)] lint.
@mockersf
Copy link
Member

mockersf commented Jan 2, 2022

For the people with ongoing doc PRs (@Sheepyhead, @james7132, @KDecay, @NiklasEi I think?), could you check your PR with the clippy lint clippy::doc_markdown?

It should be enabled soon, there was a first pass to fix issues but it's not yet finished, check #3457 for more details. It could be painful to re-introduce issues.

@ghost
Copy link

ghost commented Jan 2, 2022

... could you check your PR with the clippy lint clippy::doc_markdown?

bevy_dylib

I just checked this for my PR #3515 that updated the documentation of bevy_dylib.
I ran the following command without any errors:

cargo clippy -p bevy_dylib --no-deps --all-targets --all-features -- -D warnings -A clippy::type_complexity -W clippy::doc_markdown

bevy_math / bevy_ui

I also have the PR #3503 that is not completely done yet and updates the documentation of bevy_math and parts of bevy_ui. I will run the following commands when the PR is ready from my end:

cargo clippy -p bevy_math --no-deps --all-targets --all-features -- -D warnings -A clippy::type_complexity -W clippy::doc_markdown

cargo clippy -p bevy_ui --no-deps --all-targets --all-features -- -D warnings -A clippy::type_complexity -W clippy::doc_markdown

@Sheepyhead
Copy link
Contributor

could you check your PR with the clippy lint clippy::doc_markdown

For the record I find this lint extremely obvoxious, as it assumes anything in a doc with CamelCase is supposed to be backticked

bors bot pushed a commit that referenced this issue Jan 2, 2022
…core and enable warning on missing docs. (#3521)

This PR is part of the issue #3492.

# Objective

 - Clean up dead code in `bevy_core`.
 - Add and update the `bevy_core` documentation to achieve a 100% documentation coverage.
 - Add the #![warn(missing_docs)] lint to keep the documentation coverage for the future.

# Solution

 - Remove unused `Bytes`, `FromBytes`, `Labels`, and `EntityLabels` types and associated systems.
 - Made several types private that really only have use as internal types, mostly pertaining to fixed timestep execution.
 - Add and update the bevy_core documentation.
 - Add the #![warn(missing_docs)] lint.

# Open Questions

Should more of the internal states of `FixedTimestep` be public? Seems mostly to be an implementation detail unless someone really needs that fixed timestep state.
@james7132
Copy link
Member

Updated my PRs to pass the docs_markdown lints.

@adsick
Copy link
Contributor

adsick commented Mar 14, 2023

@james7132
Copy link
Member

Updating the table we have on the current status of this issue:

Crate Doc Coverage
bevy_a11y 100%
bevy_animation 100%
bevy_app 100%
bevy_asset 100%
bevy_audio 100%
bevy_core_pipeline 22.3%
bevy_core 100%
bevy_derive 57.1%
bevy_diagnostic 50%
bevy_dylib 100%
bevy_dynamic_plugin 42.9%
bevy_ecs_macros 40%
bevy_ecs 79.5%
bevy_encase_derive 0%
bevy_gilrs 0%
bevy_gltf 16.7%
bevy_hierarchy 100%
bevy_input 90.6%
bevy_internal 100%
bevy_log 100%
bevy_macro_utils 28.6%
bevy_math 100%
bevy_mikktspace 90%
bevy_pbr 37.4%
bevy_ptr 100%
bevy_reflect_derive 88.9%
bevy_reflect 61.7%
bevy_render_macros 25%
bevy_render 48.2%
bevy_scene 22.1%
bevy_sprite 30.8%
bevy_tasks 100%
bevy_text 35.8%
bevy_time 78.8%
bevy_transform 100%
bevy_ui 74.8%
bevy_utils_macros 0%
bevy_utils 100%
bevy_window 97.8%
bevy_winit 56%

cart pushed a commit that referenced this issue Mar 21, 2023
# Objective

- [x] Add documentation comments to `bevy_winit`
- [x] Add `#![warn(missing_docs)]` to `bevy_winit`.

Relates to #3492

---------

Co-authored-by: François <[email protected]>
@targrub
Copy link
Contributor

targrub commented Apr 3, 2023

Taking the first crate on the list currently missing documentation, bevy_core_pipeline, my best judgement (based mostly on those who originally checked in the code in its present location) is that those most familiar with the code, and thus best situated to add the requested documentation, are the following:
@cart --
crates/bevy_core_pipeline/src/blit/mod.rs
crates/bevy_core_pipeline/src/clear_color.rs
crates/bevy_core_pipeline/src/core_2d/camera_2d.rs
crates/bevy_core_pipeline/src/core_2d/main_pass_2d_node.rs
crates/bevy_core_pipeline/src/core_2d/mod.rs
crates/bevy_core_pipeline/src/core_3d/camera_3d.rs
crates/bevy_core_pipeline/src/core_3d/main_opaque_pass_3d_node.rs
crates/bevy_core_pipeline/src/core_3d/main_transparent_pass_3d_node.rs
crates/bevy_core_pipeline/src/core_3d/mod.rs
crates/bevy_core_pipeline/src/lib.rs
crates/bevy_core_pipeline/src/msaa_writeback.rs
@JMS55 --
crates/bevy_core_pipeline/src/bloom/mod.rs
crates/bevy_core_pipeline/src/bloom/settings.rs
crates/bevy_core_pipeline/src/taa/mod.rs
@Elabajaba --
crates/bevy_core_pipeline/src/contrast_adaptive_sharpening/mod.rs
crates/bevy_core_pipeline/src/contrast_adaptive_sharpening/node.rs
@jakobhellermann --
crates/bevy_core_pipeline/src/fullscreen_vertex_shader/mod.rs
crates/bevy_core_pipeline/src/tonemapping/mod.rs
crates/bevy_core_pipeline/src/tonemapping/node.rs
crates/bevy_core_pipeline/src/upscaling/mod.rs
crates/bevy_core_pipeline/src/upscaling/node.rs
@DGriffin91 --
crates/bevy_core_pipeline/src/fxaa/mod.rs
crates/bevy_core_pipeline/src/fxaa/node.rs
@IceSentry --
crates/bevy_core_pipeline/src/prepass/mod.rs
crates/bevy_core_pipeline/src/prepass/node.rs

My apologies if any my guesses were wrong.

@JMS55
Copy link
Contributor

JMS55 commented Aug 13, 2023

My plan is to get to renderer documentation (bevy_render, bevy_core_pipeline, bevy_pbr) sometime soon. I have a couple of open PRs I want to resolve first though.

github-merge-queue bot pushed a commit that referenced this issue Aug 19, 2023
# Objective

Complete the documentation of `bevy_input` (#3492).

This PR is part of a triptych of PRs :
- #9467
- #9469

## Solution

Add missing documentation on items in `lib.rs`.

---------

Co-authored-by: Pascal Hertleif <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue Aug 19, 2023
# Objective

Complete the documentation of `bevy_input` (#3492).

This PR is part of a triptych of PRs :
- #9467
- #9468

## Solution

Add missing documentation on items in `gamepad.rs`

---------

Co-authored-by: Pascal Hertleif <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue Aug 23, 2023
# Objective

Complete the documentation of `bevy_input` (#3492).

This PR is part of a triptych of PRs :
- #9468
- #9469

## Solution

Add documentation on modules in `bevy_input`.
@Kanabenki
Copy link
Contributor

Kanabenki commented Sep 28, 2023

As I started taking a shot a completing the docs for some of the crates, I thought it would be good to have some updated info on the global documentation progress. I only added PR links for those that added the warn(missing_doc) attribute once a crate doc was complete. Here's what I got:

github-merge-queue bot pushed a commit that referenced this issue Feb 3, 2024
# Objective

Currently the `missing_docs` lint is allowed-by-default and enabled at
crate level when their documentations is complete (see #3492).
This PR proposes to inverse this logic by making `missing_docs`
warn-by-default and mark crates with imcomplete docs allowed.

## Solution

Makes `missing_docs` warn at workspace level and allowed at crate level
when the docs is imcomplete.
tjamaan pushed a commit to tjamaan/bevy that referenced this issue Feb 6, 2024
# Objective

Currently the `missing_docs` lint is allowed-by-default and enabled at
crate level when their documentations is complete (see bevyengine#3492).
This PR proposes to inverse this logic by making `missing_docs`
warn-by-default and mark crates with imcomplete docs allowed.

## Solution

Makes `missing_docs` warn at workspace level and allowed at crate level
when the docs is imcomplete.
github-merge-queue bot pushed a commit that referenced this issue Feb 22, 2024
# Objective

- Some members of `bevy_dynamic_plugin` are not documented.
- Part of #3492.

## Solution

- Add documentation to members missing it in `bevy_dynamic_plugin`.
- Update existing documentation for clarity and formatting.

---

## Changelog

- Completely document `bevy_dynamic_plugin`.

---------

Co-authored-by: Alice Cecile <[email protected]>
Co-authored-by: James Liu <[email protected]>
ameknite pushed a commit to ameknite/bevy that referenced this issue Feb 22, 2024
# Objective

- Some members of `bevy_dynamic_plugin` are not documented.
- Part of bevyengine#3492.

## Solution

- Add documentation to members missing it in `bevy_dynamic_plugin`.
- Update existing documentation for clarity and formatting.

---

## Changelog

- Completely document `bevy_dynamic_plugin`.

---------

Co-authored-by: Alice Cecile <[email protected]>
Co-authored-by: James Liu <[email protected]>
msvbg pushed a commit to msvbg/bevy that referenced this issue Feb 26, 2024
# Objective

- Some members of `bevy_dynamic_plugin` are not documented.
- Part of bevyengine#3492.

## Solution

- Add documentation to members missing it in `bevy_dynamic_plugin`.
- Update existing documentation for clarity and formatting.

---

## Changelog

- Completely document `bevy_dynamic_plugin`.

---------

Co-authored-by: Alice Cecile <[email protected]>
Co-authored-by: James Liu <[email protected]>
msvbg pushed a commit to msvbg/bevy that referenced this issue Feb 26, 2024
# Objective

- Some members of `bevy_dynamic_plugin` are not documented.
- Part of bevyengine#3492.

## Solution

- Add documentation to members missing it in `bevy_dynamic_plugin`.
- Update existing documentation for clarity and formatting.

---

## Changelog

- Completely document `bevy_dynamic_plugin`.

---------

Co-authored-by: Alice Cecile <[email protected]>
Co-authored-by: James Liu <[email protected]>
@BD103 BD103 added D-Straightforward Simple bug fixes and API improvements, docs, test and examples and removed D-Trivial Nice and easy! A great choice to get started with Bevy labels Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Docs An addition or correction to our documentation C-Tracking-Issue An issue that collects information about a broad development initiative D-Straightforward Simple bug fixes and API improvements, docs, test and examples
Projects
None yet
Development

No branches or pull requests