Skip to content

[Merged by Bors] - Hide docs for concrete impls of Fetch, FetchState, and SystemParamState#4250

Closed
james7132 wants to merge 11 commits intobevyengine:mainfrom
james7132:hide-docs
Closed

[Merged by Bors] - Hide docs for concrete impls of Fetch, FetchState, and SystemParamState#4250
james7132 wants to merge 11 commits intobevyengine:mainfrom
james7132:hide-docs

Conversation

@james7132
Copy link
Copy Markdown
Member

@james7132 james7132 commented Mar 18, 2022

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Mar 18, 2022
@james7132 james7132 added C-Docs An addition or correction to our documentation A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels Mar 18, 2022
@mockersf
Copy link
Copy Markdown
Member

would the #[doc(hidden)] hides them from the https://dev-docs.bevyengine.org/bevy/ecs/query/trait.ReadOnlyFetch.html#implementors page?

While this page is not very... friendly, it's still useful to list all the struct that implement that trait.

@james7132
Copy link
Copy Markdown
Member Author

would the #[doc(hidden)] hides them from the https://dev-docs.bevyengine.org/bevy/ecs/query/trait.ReadOnlyFetch.html#implementors page?

While this page is not very... friendly, it's still useful to list all the struct that implement that trait.

It seems like it would, but the supported WorldQuery implementors are still there, which is what end users will end up interacting with anyway. The exact implementors for Fetch, ReadOnlyFetch, and FetchState are, IMO, largely an implementation detail that really doesn't need to be publicly documented.

Copy link
Copy Markdown
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I like hiding these, but I would prefer to leave the doc comments in place. They're useful guidance for developers ("what the heck is this?"), and there's no cost to keeping them.

Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I like this change. Nothing got hidden that end users are interacting with and the docs will be a lot less noisy this way. I was quite impressed how noisy the docs look currently.

@james7132 james7132 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 21, 2022
@alice-i-cecile
Copy link
Copy Markdown
Member

bors r+

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Mar 21, 2022

Build failed:

@ghost
Copy link
Copy Markdown

ghost commented Mar 21, 2022

Looks like https://wiki.ubuntu.com/WSL is down. CI is also failing on #4275 and I also can't access the site.

@james7132
Copy link
Copy Markdown
Member Author

bors retry

@bors bors bot changed the title Hide docs for concrete impls of Fetch, FetchState, and SystemParamState [Merged by Bors] - Hide docs for concrete impls of Fetch, FetchState, and SystemParamState Mar 21, 2022
@bors bors bot closed this Mar 21, 2022
@james7132 james7132 deleted the hide-docs branch June 22, 2022 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events 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.

3 participants