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

[Merged by Bors] - Clarify the behaviour of iter_many in the docs #5973

Closed
wants to merge 2 commits into from

Conversation

tim-blackbird
Copy link
Contributor

Add the following message:

Items are returned in the order of the list of entities.
Entities that don't match the query are skipped.

Additionally, the docs in iter.rs and state.rs were updated to match those in query.rs.

@james7132 james7132 added C-Docs An addition or correction to our documentation A-ECS Entities, components, systems, and events labels Sep 13, 2022
/// This can only return immutable data (mutable data will be cast to an immutable form).
/// See [`Self::iter_many_mut`] for queries that contain at least one mutable component.
/// Items are returned in the order of the list of entities.
/// Entities that don't match the query are skipped.
Copy link
Contributor

Choose a reason for hiding this comment

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

What about linking to QueryManyIter instead of duplicating text? That would remove the risk of divergent evolution.

Copy link
Contributor

@rparrett rparrett Sep 15, 2022

Choose a reason for hiding this comment

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

I agree, but didn't we discuss/do the opposite in #4989?

Copy link
Contributor

Choose a reason for hiding this comment

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

I really think not. I always try my best to keep API docs in their place and avoid duplicating them.

Copy link
Contributor

Choose a reason for hiding this comment

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

My bad, I misremembered how that conversation went down. I still wish we could stick to a single convention with regard to whether the meat of the docs live on the iterator type or the method, but as long as everything's linked up it's not worth fussing about.

Copy link
Contributor

Choose a reason for hiding this comment

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

The rule of thumb is to document as close as possible to the implementation, so the iterator. The problem may be that the closest item is not very visible. In this case, good linking practices help a lot.

@rparrett
Copy link
Contributor

Chiming in briefly to say that this is information I really wanted when I was browsing these docs recently. Will look a bit closer later.

@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 Oct 19, 2022
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Oct 24, 2022
Add the following message:
```
Items are returned in the order of the list of entities.
Entities that don't match the query are skipped.
```

Additionally, the docs in `iter.rs` and `state.rs` were updated to match those in `query.rs`.

Co-authored-by: devil-ira <justthecooldude@gmail.com>
@bors bors bot changed the title Clarify the behaviour of iter_many in the docs [Merged by Bors] - Clarify the behaviour of iter_many in the docs Oct 24, 2022
@bors bors bot closed this Oct 24, 2022
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
Add the following message:
```
Items are returned in the order of the list of entities.
Entities that don't match the query are skipped.
```

Additionally, the docs in `iter.rs` and `state.rs` were updated to match those in `query.rs`.

Co-authored-by: devil-ira <justthecooldude@gmail.com>
@tim-blackbird tim-blackbird deleted the iter-many-docs branch November 7, 2022 21:07
Pietrek14 pushed a commit to Pietrek14/bevy that referenced this pull request Dec 17, 2022
Add the following message:
```
Items are returned in the order of the list of entities.
Entities that don't match the query are skipped.
```

Additionally, the docs in `iter.rs` and `state.rs` were updated to match those in `query.rs`.

Co-authored-by: devil-ira <justthecooldude@gmail.com>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
Add the following message:
```
Items are returned in the order of the list of entities.
Entities that don't match the query are skipped.
```

Additionally, the docs in `iter.rs` and `state.rs` were updated to match those in `query.rs`.

Co-authored-by: devil-ira <justthecooldude@gmail.com>
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.

6 participants