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

Docstring updates #921

Merged
merged 24 commits into from
Jan 5, 2025
Merged

Conversation

isaacbrodsky
Copy link
Collaborator

@isaacbrodsky isaacbrodsky commented Sep 30, 2024

This updates the doc strings in a few places to be consistent with the actual parameters. I also changed Doxygen settings to improve that a little, e.g. to include the public API header.

Based on #919, #920.

@coveralls
Copy link

coveralls commented Sep 30, 2024

Coverage Status

coverage: 98.784%. remained the same
when pulling 51df30a on isaacbrodsky:compact_all_res1_tes-docs
into ae31177 on uber:master.

@isaacbrodsky isaacbrodsky marked this pull request as ready for review October 20, 2024 16:43
@isaacbrodsky
Copy link
Collaborator Author

This should be ready for review now.

@@ -127,7 +127,7 @@ H3Error H3_EXPORT(areNeighborCells)(H3Index origin, H3Index destination,
* destination
* @param origin The origin H3 hexagon index
* @param destination The destination H3 hexagon index
* @return The directed edge H3Index, or H3_NULL on failure.
* @param out Output: The directed edge H3Index.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we include the @return still for the H3Error path?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Eg, just copy this from further below:

Suggested change
* @param out Output: The directed edge H3Index.
* @param out Output: The directed edge H3Index.
* @returns E_SUCCESS on success, or another value on error

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was most focused on ensuring the docstring wasn't incorrect, rather than completing it. That seems like a good addition.

@isaacbrodsky isaacbrodsky merged commit 333c2bc into uber:master Jan 5, 2025
38 checks passed
@isaacbrodsky isaacbrodsky deleted the compact_all_res1_tes-docs branch January 5, 2025 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants