Skip to content

Conversation

joseph-gio
Copy link
Member

@joseph-gio joseph-gio commented Aug 12, 2023

Objective

  • Add documentation to undocumented items in bevy_time.
  • Add a warning for undocumented items.

@joseph-gio joseph-gio added C-Docs An addition or correction to our documentation A-Time Involves time keeping and reporting labels Aug 12, 2023
@alice-i-cecile
Copy link
Member

Progress towards #3492

@Selene-Amanita
Copy link
Member

Selene-Amanita commented Aug 12, 2023

Not introduced by this PR but can be fixed by it, wrong backticks in https://docs.rs/bevy/0.11.0/bevy/time/fixed_timestep/struct.FixedTime.html#method.expend:
[`Err(FixedUpdateError`)]
(should probably link to FixedUpdateError not Err too)

(the doc of expend is not clear at all either, which I guess is fine since it's not supposed to be used by a lot of people, only people who make tests or make their own runner, but maybe the doc should say that then?)

Copy link
Member

@Selene-Amanita Selene-Amanita left a comment

Choose a reason for hiding this comment

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

The doc on this page is great: https://docs.rs/bevy_time/0.11.0/bevy_time/fixed_timestep/index.html but FixedTime and run_fixed_update_schedule don't link back to it. Since FixedUpdate links to run_fixed_update_schedule, I think it would be great to link back to the central doc on fixed time, people (me at least) don't always think to check module documentation.

@joseph-gio
Copy link
Member Author

Thank you for the review. I believe I have addressed your points.

Copy link
Member

@Selene-Amanita Selene-Amanita left a comment

Choose a reason for hiding this comment

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

Besides a small typo looks good to me!

@Selene-Amanita Selene-Amanita added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Aug 13, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 14, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 14, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 14, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 14, 2023
@GuillaumeGomez
Copy link
Contributor

To help you with missing docs, you can add #![deny(missing_docs)]. :)

@alice-i-cecile
Copy link
Member

@GuillaumeGomez a warn equivalent was added in lib.rs :) Like we discussed in the linked issue, warn is less intrusive for development, but still blocks CI.

@GuillaumeGomez
Copy link
Contributor

Nice! There is also rustdoc::missing_doc_code_examples which could be helpful but I think it's nightly only. Bevy docs is really lacking examples. ^^'

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 15, 2023
Merged via the queue into bevyengine:main with commit f99dcad Aug 15, 2023
@joseph-gio joseph-gio deleted the document-time branch August 17, 2023 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Time Involves time keeping and reporting C-Docs An addition or correction to our documentation S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants