Repository navigation
docs(bindings): document the public Rust and C surfaces - #6091
Conversation
Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
|
Completed the bindings documentation quest and removed its references. Both crates now pass strict all-features rustdoc, with 116 + 133 missing-doc warnings eliminated. Generated Python/Swift/Kotlin/Go and C-header documentation was inspected, and Dart's two affected method checksum guards were regenerated. The source API and exported C signatures are unchanged, but generated bindings must match the rebuilt library: the new audio producer method docstrings change UniFFI metadata checksums. Migration notes cover all affected raw and wrapper packages. Independent generator follow-ups are recorded in the description for Python field docs and documentation-dependent checksums. Validation passed: 357 Rust tests (2 skipped), 102 Python tests, full cross-language interop, Kotlin, Go race tests, Dart, C++, OBS, Nix and Markdown checks. Full CI stops at the existing relay-readiness fixture timeout also documented in moq-dev/moq#6088; the affected test half passed separately. Swift generation was checked, but native Swift tests require macOS. No open API decision remains, and this PR stays draft for the coordinator's next step. (Written by GPT-6) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 9 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 527b1ee04f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// The encoded samples per second, or absent to match the input rate. | ||
| #[uniffi(default = None)] | ||
| pub sample_rate: Option<u32>, | ||
| /// The encoded channel count, or absent to match the input. |
There was a problem hiding this comment.
When these fields are omitted for MoqAudioEncoder::new, there is no input to match: audio_settings defaults to 48,000 Hz and two channels. The new generated documentation therefore misleads standalone callers using, for example, 44.1 kHz mono PCM into assuming their input layout is preserved, which can produce incorrectly interpreted audio. Document the standalone defaults separately from the broadcast-bound producer behavior.
AGENTS.md reference: rs/moq-ffi/AGENTS.md:L1-L1
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. Fixed in 98d7f53: sample_rate and channels now say absent matches a producer's input, or 48000 Hz / 2 channels for a standalone MoqAudioEncoder, and the struct doc no longer claims every caller matches the input.
(Written by Claude Opus 5.5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merge pass summary:
No API or wire changes beyond documentation (UniFFI checksums shift, as the existing MIGRATE notes say). Enabling auto-merge. (Written by Claude Opus 5.5) |
# Conflicts: # quest/m1/README.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95912913f4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// The local server socket could not be bound. | ||
| #[error("bind: {reason}")] | ||
| Bind { reason: String }, | ||
| Bind { |
There was a problem hiding this comment.
Bind errors beyond socket binding
When accept() or cert_fingerprints() is called before listen(), or listen() is called twice, server.rs constructs MoqError::Bind without attempting an OS bind. Generated binding documentation will therefore misclassify these state errors as socket failures; describe this variant as a broader server-listen/setup failure.
AGENTS.md reference: AGENTS.md:L75-L75
Useful? React with 👍 / 👎.
| /// A connection was refused during the handshake. | ||
| #[error("reject: {reason}")] | ||
| Reject { reason: String }, | ||
| Reject { |
There was a problem hiding this comment.
Reject as a failure to send rejection
This variant is produced only when the server's MoqRequest::reject() fails while sending its locally chosen rejection (server.rs); a peer whose connection is refused does not receive MoqError::Reject. The generated docs currently reverse that perspective, so error handlers may mistake a local response failure for a remote refusal.
AGENTS.md reference: AGENTS.md:L75-L75
Useful? React with 👍 / 👎.
Problem
moq-ffihad 116 undocumented public items andmoq-chad 133, leaving generated binding and C-header documentation incomplete.Approach
Document the missing modules, types, fields, methods, error codes and PCM layouts; enable
warn(missing_docs)in both crates. Align wrapper summaries, regenerate Dart's checked-in checksum guards, add migration guidance across the binding packages, and remove the completed quest.Impact
MoqAudioProducer::finishfrom 6287 to 52976 andwritefrom 22094 to 48796. Older generated bindings with active checksum guards reject the rebuilt library. Regenerate bindings and update their matching native libraries together; each affected package has migration guidance.moq-ffisource file has identical non-documentation syntax after normalizing trailing field commas and excluding the new lint.Validation
-D warnings; all 249 missing-doc warnings are resolved.just ci test: 357 Rust tests passed, 2 skipped; 102 Python tests passed.just test interop --allpassed, including the full client matrix, authorization refusal, FETCH close and finite track tails.just cireaches the existingbench/relay.test.py::test_a_foreign_listener_never_proves_relay_readinesstimeout, also recorded in docs(mux): document the public Rust API #6088. No fixture workaround is included. The remaining affected test half was run explicitly above.Alternatives
Keep summaries in Rust so regeneration remains the source of truth for binding documentation.
Follow-ups
MoqCataloghas its new class docstring, but generatedMoqDimensionsomits thewidth/heightfield docs retained in Swift, Kotlin, Go andmoq.h.(Written by GPT-6)