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] - Document collide args #2721

Closed
wants to merge 2 commits into from

Conversation

rukai
Copy link
Contributor

@rukai rukai commented Aug 24, 2021

Fixes #2720 (comment)

Possibly we should omit the extra bit in brackets, not sure if its warranted.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Aug 24, 2021
@rukai rukai force-pushed the document_collide branch 2 times, most recently from 2d7bd41 to 5e04005 Compare August 24, 2021 13:29
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
@rukai
Copy link
Contributor Author

rukai commented Aug 24, 2021

I like that suggestion! Makes it very clear how to use.

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation C-Examples An addition or correction to our examples and removed S-Needs-Triage This issue needs to be labelled labels Aug 24, 2021
@alice-i-cecile alice-i-cecile 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 24, 2021
@cart
Copy link
Member

cart commented Aug 24, 2021

bors r+

@Weibye
Copy link
Contributor

Weibye commented Aug 24, 2021

Would something like

/// # Arguments
/// 
/// * `a_pos` - Center-position of the first rectangle, 
/// * `a_size` - Size of the first rectangle in height; width
/// * `b_pos` - Center-position of the second rectangle
/// * `b_size` - Size of the second rectangle in height; width

be too much or would it be useful?

@cart
Copy link
Member

cart commented Aug 24, 2021

We don't see "structured" argument docs like that very often in rust docs. Generally they are documented free-form in sentences. I think there are arguments to be made for both approaches, but given that this goes against a pattern we currently use, it should probably be discussed separately.

bors bot pushed a commit that referenced this pull request Aug 24, 2021
Fixes #2720 (comment)

Possibly we should omit the extra bit in brackets, not sure if its warranted.
@bors bors bot changed the title Document collide args [Merged by Bors] - Document collide args Aug 24, 2021
@bors bors bot closed this Aug 24, 2021
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-Examples An addition or correction to our examples 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.

collide_aabb is completely broken
4 participants