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

Document bevy_gizmos #8186

Merged
merged 10 commits into from
Mar 28, 2023
Merged

Document bevy_gizmos #8186

merged 10 commits into from
Mar 28, 2023

Conversation

tim-blackbird
Copy link
Contributor

@tim-blackbird tim-blackbird commented Mar 23, 2023

Objective

Fix #8179

Solution

  • Added #![warn(missing_docs)] and document all public items. All methods on Gizmos have doc examples.
  • Expanded the docs on the module/crate. Some unfortunate duplication there :/
  • Moved the methods from GizmoBuffer to be directly on Gizmos and made GizmoBuffer private. This means the methods on Gizmos will show up on its doc page.

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-Gizmos Visual editor and debug gizmos labels Mar 24, 2023
@alice-i-cecile
Copy link
Member

Once there's module docs this LGTM. Solid work so far, and I like the gratuitous use of doc examples.

@tim-blackbird tim-blackbird marked this pull request as ready for review March 24, 2023 10:07
@james7132 james7132 added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Mar 28, 2023
@james7132 james7132 added this to the 0.11 milestone Mar 28, 2023
@github-actions
Copy link
Contributor

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

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

Docs look good to me. The removal of GizmosBuffer as a pub struct is a breaking change though, as is the change from a alias to a actual struct.

crates/bevy_gizmos/src/lib.rs Show resolved Hide resolved
Co-authored-by: James Liu <[email protected]>
@tim-blackbird
Copy link
Contributor Author

Thanks for the review!
There are no breaking changes here since this API was added after 0.10 was released.

@james7132 james7132 removed the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Mar 28, 2023
@james7132
Copy link
Member

There are no breaking changes here since this API was added after 0.10 was released.

Ah yeah, you're right. Thanks for noting that.

@mockersf mockersf 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 Mar 28, 2023
@mockersf mockersf added this pull request to the merge queue Mar 28, 2023
Merged via the queue into bevyengine:main with commit 0893852 Mar 28, 2023
@tim-blackbird tim-blackbird deleted the document-gizmos branch April 4, 2023 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Gizmos Visual editor and debug gizmos 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.

Document bevy::gizmos
4 participants