-
Notifications
You must be signed in to change notification settings - Fork 297
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 Style Guide and API Design Guide #1584
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great @j9liu, thanks for writing it up! Sorry for the large number of small comments here.
Documentation/style-guide.md
Outdated
|
||
Before diving into this guide, we recommend the following: | ||
|
||
1. Browse Cesium Native's [C++ style guide](https://github.com/CesiumGS/cesium-native/blob/main/doc/style-guide.md) to familiarize yourself with fundamental principles and expectations for C++ development. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This URL is invalid because the file moved recently. @azrogers we can now use the tagfile to create a reliable link to the published version of the docs, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! Might need to grab the latest version of the tag file (from https://cesium.com/learn/cesium-native/ref-doc/cesium-native.tag), but you should be able to replace this with \ref style-guide "C++ style guide"
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be okay to just link to the guide as published online? (https://cesium.com/learn/cesium-native/ref-doc/style-guide.html)
Personally I still look at Github for our documentation so I appreciate when things can also work well on Github. But I'm willing to concede and use \ref
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or you could do <!--! \ref style-guide "C++ style guide" --><!--! \cond DOXYGEN_EXCLUDE !-->[C++ style guide](https://cesium.com/learn/cesium-native/ref-doc/style-guide.html)<!--! \endcond -->
. But, uh, just linking to it is probably better.
Co-authored-by: Kevin Ring <[email protected]>
No apologies @kring, thank you for the thorough review! I think I addressed all of your comments, except for the image resize that's breaking Doxygen. I'm happy just using Markdown and living with the bigger image. I turned some of your comments into short new sections; let me know if I got everything right! |
- Disable STRIP_CODE_COMMENTS so the doxygen comments in the code samples show up. - Fix broken link to functions section. - Remove broken link to old "Developing UObjects" doc.
Thanks @j9liu! I've disabled |
Fixes #1540, #1541
Probably not exhaustive, but hopefully a good start. There's the possibility of merging these into a single Coding Guide (similar to CesiumJS), but I'm not sure what the call is. I think there's enough separation for now, and we can always continue to add to these docs in the future.