Doxygen second pass: coverage, improvements, fixes, etc.#142
Doxygen second pass: coverage, improvements, fixes, etc.#142alan-george-lk wants to merge 28 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ntain permissions' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…ntain permissions' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
… into feature/more_docs
… into feature/more_docs
stephen-derosa
left a comment
stephen-derosa
left a comment
There was a problem hiding this comment.
just reviewed agents/code changes. Still need to go through the GHA and docs/ work but lookig great!
There was a problem hiding this comment.
maybe just a me thing, but i originally thought this was generating docs in the code, consider renaming to build-doxygen
There was a problem hiding this comment.
I went with generate-docs for both this and the Workflow stage because the existing one was called publish-docs. I'm open to changing it to build-docs but was being overly pedantic about having the workflow be adjacently indexed alphabetically to the publish-docs stage. And it's not really a build, it's a codegen
There was a problem hiding this comment.
Example output from Doxygen itself in this stage:
Generating style sheet...
Generating search indices...
Generating example documentation...
Generating file sources...
Generating code for file include/livekit/audio_frame.h...
Generating code for file include/livekit/audio_processing_module.h...
Generating code for file include/livekit/audio_source.h...
Generating code for file include/livekit/audio_stream.h...
| /// stream->close(); // optional, called automatically in destructor | ||
| class LIVEKIT_API VideoStream { | ||
| public: | ||
| /// @brief Options for creating a decoded video frame stream. |
There was a problem hiding this comment.
decoded or encoded?
There was a problem hiding this comment.
This is the outer class doc:
/// Represents a pull-based stream of decoded video frames coming from
/// a remote (or local) LiveKit track. Similar to AudioStream, but for video.
Which existed before I started mucking around. I'm not sure 100%
There was a problem hiding this comment.
You can omit @breif since you have JAVADOC_AUTOBRIEF enabled in the config.
| class OwnedTrackPublication; | ||
| } | ||
|
|
||
| /// @brief Publication metadata for a remotely published media track. |
There was a problem hiding this comment.
is this actually metadata? looks like OwnedTrackPublication also has a an OwnedTrackPublication object
There was a problem hiding this comment.
Good catch, fixed
| } | ||
|
|
||
| /// @brief Publication metadata for a locally published media track. | ||
| class LIVEKIT_API LocalTrackPublication : public TrackPublication { |
There was a problem hiding this comment.
same metadata comment
There was a problem hiding this comment.
Also good catch, fixed
stephen-derosa
left a comment
stephen-derosa
left a comment
There was a problem hiding this comment.
few small commands but lgtm!
| paths: | ||
| - include/** | ||
| - src/** | ||
| - examples/** |
There was a problem hiding this comment.
idt this dir exists?
| upload_artifact: true | ||
| # Suffix with run_id so concurrent publish runs cannot collide on the | ||
| # artifact namespace within the same repository. | ||
| artifact_name: livekit-cpp-docs-${{ github.run_id }} |
There was a problem hiding this comment.
run_id==commit hash? if not we should consider using commit hash
There was a problem hiding this comment.
Nope, this is the integer incrementing count (see actions tab). Just needs to be unique such that if for some reason two releases are pushing docs (say we back-patched an old tag in addition to a new one simultaneously), they wouldn't conflict.
This is just the temporary artifact name between the generate and publish stages in the actions space, not in S3 or anywhere permanent. Open to SHA if you prefer
| LATEST_PREFIX="s3://livekit-docs/client-sdk-cpp" | ||
|
|
||
| # Upload immutable versioned docs, then refresh rolling latest. | ||
| aws s3 cp "$DOCS_DIR"/ "$VERSIONED_PREFIX" --recursive |
There was a problem hiding this comment.
i always hated wrestling aws creds, glad we have it for this!
| @@ -1,9 +1,7 @@ | |||
| # LiveKit C++ Client SDK | |||
| # Overview | |||
There was a problem hiding this comment.
i think other SDKs have LiveKit <lang> Client SDK, no?
There was a problem hiding this comment.
i am thinking this is in location of the SS atttatched, if not no worries:
There was a problem hiding this comment.
I'll reference this JS one and match phrasing -- the reason I removed this is previously we had:
- LiveKit C++ SDK
- LiveKit C++ Client SDK
<child items>
- LiveKit C++ Client SDK
On the side navigation tree (see above screenshots in PR body) which were ugly. That's what this change did, made the second one just Overview
any way to get rid of the `9417...` trailing `v0.4.0` ?
|
4 participants
Added:
scripts/generate-docs.sh: like theclang-*tools before, helper script for generating Doxygen locally and in CIgenerate-docs.ymlworkflow stage that validates documentation generates and has no errors, which will catch broken refs, incomplete docs, etc.Changes:
docs/todocs/doxygen. This was a strategic decision such thatdocs/could also hold plain.mdfiles for developer/design reference, such as the Rust SDK/common repo convention(rendering changes below)
LiveKit C++ SDKwas duplicated in the tree)README.mdembedding into the SDK. The GitHub light/dark mode logo rendered wrong and would have been cumbersome to edit programmatically during docs build to get rightFixes:
Class Listview have entriesBefore
Tree (redundant and no version):
Examples (broken links):
After
Tree (less redundant and has version)
Examples (expanded and fixed links):