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

Edit doxygen comments for consistency and style #1087

Merged

Conversation

cary-ilm
Copy link
Member

  • Change @sa to \ref where it occurred inline in a sentence. @sa
    expands to a "See also" section, not suitable in an inline reference.
  • Make function description tense imperative: "Set the...", "Return the...", etc
  • Mark parameters with @p and functions with \ref.
  • Add periods after all sentences except @brief summaries.
  • Capitalize all sentences.

This is done only for the functions/symbols marked as EXR_EXPORT (thus part of the public API).

Signed-off-by: Cary Phillips cary@ilm.com

* Change @sa to \ref where it occurred inline in a sentence. @sa
  expands to a "See also" section, not suitable in an inline reference.
* Make function description tense imperative: "Set the...", "Return the...", etc
* Mark parameters with @p and functions with \ref.
* Add periods after all sentences except @brief summaries.
* Capitalize all sentences.

Signed-off-by: Cary Phillips <cary@ilm.com>
Copy link
Contributor

@kdt3rd kdt3rd left a comment

Choose a reason for hiding this comment

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

Thanks - I've been using sa wrong all along then, or my brain only remembers old-school doxygen. Do you think I need to make another pass at adding detail to the doxygen? Or only spend time on the restructured text file instead?

@cary-ilm
Copy link
Member Author

Half of your uses of @sa were appropriate, where they naturally expand into a "See " section, but the other half were used incorrectly.

Take a look at #1088, which sets up the rst/readthedocs structure. The high-level description goes in OpenEXRCoreAPI.rst, but that file also explicitly pulls in doxygen comments from specific functions via ".. doxygenfunction::". I pulled in documentation for each function marked as EXR_EXPORT, since that defines the public API. Plus other associated enums and structs. That section is a big of a jumble as is, but it's a start. I took a stab at organizing the functions by purpose, but that could use some more work. Each of these sections could use a prose intro as well. The whole section would work better split out into separate files/pages as well.

We should beef up the description of each function, and that can go in doxygen. If there are other helpful sections in doxygen that we can pull into OpenEXRCoreAPI.rst, that's fine, but we should also be free to put descriptions only in the .rst, since we have more organizational control there.

@cary-ilm cary-ilm merged commit 9150dc1 into AcademySoftwareFoundation:master Jul 14, 2021
@cary-ilm cary-ilm added the v3.1.0 label Sep 2, 2021
@cary-ilm cary-ilm deleted the doxygen-comment-edits branch November 5, 2022 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants