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

Add Documentation section to style guide #1062

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions doc/topics/style-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,56 @@ Not covered by C++ Core Guidelines.

* Test files should be prefixed with `Test`, e.g. `TestModel` instead of `ModelTests`.

## ✏️ Documentation

Not covered by C++ Core Guidelines.

Our documentation is generated by Doxygen before being published [online](https://cesium.com/learn/cesium-native/ref-doc). Therefore, any public API should be documented with Doxygen-compatible comments that use of the following tags (in the order listed):
j9liu marked this conversation as resolved.
Show resolved Hide resolved

- `@brief`: Summarize the purpose of the class, function, etc.
- `@details`: Use for lengthier explanations, after `@brief`.
j9liu marked this conversation as resolved.
Show resolved Hide resolved
- `@tparam`: Describe template parameters.
- `@param`: Denote and describe function parameters.
- `@returns`: Describe the return value of a non-`void` function.
Copy link
Contributor

Choose a reason for hiding this comment

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

I sometimes see @return (singular). I like the plural version more.

- `@exception`: Describe any exceptions that a function throws.
j9liu marked this conversation as resolved.
Show resolved Hide resolved
j9liu marked this conversation as resolved.
Show resolved Hide resolved

Additionally, make sure to:

- Put comments **above** the thing being documented (instead of inline).
- Use the `/** ... */` comment style (instead of `///`).
- Use `\ref` when referencing other classes, functions, etc. in the Cesium Native API.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would @link be a better choice? We seem to use both interchangeably and @link is more consistent with JSDoc.

Copy link
Contributor Author

@j9liu j9liu Jan 15, 2025

Choose a reason for hiding this comment

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

We initially were using @link, but I think it wasn't actually working when we generated documentation? 😦 hence #800

Copy link
Contributor

Choose a reason for hiding this comment

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

Technically @ref and @link aren't interchangeable. \ref can have an optional link text specified after the ref name, like @ref CesiumUtility::Uri "URI parsing". Whereas @link is supposed to be paired with @endlink, like @link CesiumUtility::Uri URI parsing @endlink. Because we use the JSDoc inline link tag, {@link name}, Doxygen somehow seems to understand this and doesn't require us to use @endlink. But I'd prefer to use the suggested Doxygen syntax rather than this JSDoc compatibility feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@azrogers do you know if @ref works the same as \ref? If so, I think we should stick with @ref so it matches the rest of the tags we use

Copy link
Contributor

Choose a reason for hiding this comment

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

@azrogers @j9liu thanks and good to know, I'll stop using @link and use @ref instead

Copy link
Contributor

Choose a reason for hiding this comment

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

@j9liu @ref and \ref work the same. The @ and \ are interchangeable prefixes (I accidentally used \ref where I meant to use @ref in that comment). Personally, I like to use @ in the directives at the start of the line and \ for the directives in the middle of the line (like @brief Some text with a \ref Item tag) to distinguish between inline and non-inline tags. But that's just me and it's probably best if we keep it consistent instead.

- Use proper capitalization, grammar, and spelling.
- Use back apostrophes (`) to encapsulate references to types and variable names.
- Use triple back apostrophes (```) to encapsulate any snippets of math or code.

You can optionally use `@snippet` to include examples of how functions are used in other parts of the code (usually test cases). First, surround the target code with comments containing a descriptive name:

```cpp
// In TestPlane.cpp...

TEST_CASE("Plane constructor from normal and distance") {
//! [constructor-normal-distance]
// The plane x=0
Plane plane(glm::dvec3(1.0, 0.0, 0.0), 0.0);
//! [constructor-normal-distance]
}
```

Then reference the file and the name of the snippet:

```cpp
/**
* @brief Constructs a new plane from a normal and a distance from the origin.
*
* Example:
* @snippet TestPlane.cpp constructor-normal-distance
*/
Plane(const glm::dvec3& normal, double distance);
```


> Although private classes and functions aren't required to have the same level of documentation, it never hurts to add any, especially if they have non-obvious assumptions, scope, or consequences.

## 🗂️ Other

Not covered by C++ Core Guidelines.
Expand Down