Tighten public APIs ahead of release: non_exhaustive + builders#1472
Conversation
- moq-msf: mark `Track` non_exhaustive and add `Track::new` so future CMSF/MSF field additions don't keep breaking the wire-format struct. - moq-mux: default `Fmp4::Export::new` and `Mkv::Export::new` to the `Hang` catalog format; add `with_catalog_format` for callers that need MSF (moq-cli's `--catalog` flag). - moq-relay: mark `AuthToken` non_exhaustive and refactor `Cluster` to a builder. `Cluster::new(config)` takes only `ClusterConfig`; attach the QUIC client and stats aggregator via `with_client` and `with_stats`. `StatsConfig::build(origin) -> Stats` centralizes the config-to-aggregator translation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Your plan currently allows 4 reviews/hour. Refill in 4 minutes and 28 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR refactors multiple public APIs to require explicit construction paths and builder-style initialization. The 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rs/moq-msf/src/lib.rs (1)
32-36: ⚡ Quick winUpdate documentation to reflect the new
Track::newconstructor.The current documentation suggests external code should "start from a previously produced
Track" or "use struct update syntax", but the newly addedTrack::newconstructor (lines 113-141) provides a direct fresh construction path. The documentation should mentionTrack::new(...)as the primary construction method for external callers, followed by field assignment.📝 Suggested documentation update
/// A single track in the MSF catalog. /// /// Marked `#[non_exhaustive]` because the CMSF/MSF drafts continue to grow /// optional fields. External constructors should start from a previously -/// produced `Track` (e.g. from `Catalog::from_str`) or use struct update -/// syntax against one. +/// produced `Track` (e.g. from `Catalog::from_str`), or use [`Track::new`] +/// to construct a fresh track and set fields via assignment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rs/moq-msf/src/lib.rs` around lines 32 - 36, Update the doc comment that currently advises consumers to "start from a previously produced `Track` or use struct update syntax" to instead call out the new public constructor `Track::new(...)` as the recommended primary construction path for external callers, then mention that callers can further adjust optional fields via struct update syntax or field assignment; update the sentence near the `#[non_exhaustive]` note to mention `Track::new` and keep `Catalog::from_str` as an alternative source when appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@rs/moq-msf/src/lib.rs`:
- Around line 32-36: Update the doc comment that currently advises consumers to
"start from a previously produced `Track` or use struct update syntax" to
instead call out the new public constructor `Track::new(...)` as the recommended
primary construction path for external callers, then mention that callers can
further adjust optional fields via struct update syntax or field assignment;
update the sentence near the `#[non_exhaustive]` note to mention `Track::new`
and keep `Catalog::from_str` as an alternative source when appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab5f5061-809e-4654-badf-42a46e9f6dff
📒 Files selected for processing (12)
rs/moq-cli/src/subscribe.rsrs/moq-msf/src/lib.rsrs/moq-mux/src/catalog/hang/producer.rsrs/moq-mux/src/catalog/msf/consumer.rsrs/moq-mux/src/container/fmp4/export.rsrs/moq-mux/src/container/fmp4/export_test.rsrs/moq-mux/src/container/mkv/export.rsrs/moq-mux/src/container/mkv/export_test.rsrs/moq-relay/src/auth.rsrs/moq-relay/src/cluster.rsrs/moq-relay/src/main.rsrs/moq-relay/src/stats.rs
Struct update syntax (`..base`) is also blocked by `#[non_exhaustive]` outside the defining crate, so the previous doc was misleading. Point external callers at `Track::new` + field assignment instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Reaction to the breaking changes flagged in the release-plz PR #1467. Marks wire-format and auth types
#[non_exhaustive]so additive changes stop counting as breaking, and reshapes a couple of constructors so the breaks land in this release instead of the next several.Changes
moq-msfTrackis now#[non_exhaustive]and gains aTrack::new(name, packaging)constructor. External callers (currently justmoq-mux) build a track vianew+ field assignment instead of a struct literal, but future CMSF/MSF optional fields can be added without a major bump.moq-muxFmp4::Export::new(broadcast)andMkv::Export::new(broadcast)now default toCatalogFormat::Hang. Callers that need MSF use the newwith_catalog_format(broadcast, format)constructor.moq-clisubscribe updated accordingly; tests collapse toExport::new(consumer)since they were always asking for Hang.moq-relayAuthTokenis now#[non_exhaustive]. All construction already routes throughAuthToken::unrestricted()orAuth::verify(), so nothing else moved.Clusterswitched to a small builder:Cluster::new(config)— fresh origin, no client,Stats::disabled()..with_client(client)— required whenconfig.connectis non-empty (validated inrun()with a clear error)..with_stats(stats)— replaces the no-op default.Cluster.clientis nowOption<moq_native::Client>.StatsConfig::build(origin) -> Statsencapsulates the config-to-aggregator translation, so the relay'smain.rscalls it unconditionally (it returnsStats::disabled()when the config has stats off).Test plan
cargo check -p moq-msf -p moq-mux -p moq-cli -p moq-relay --all-targetscargo test -p moq-msf -p moq-mux -p moq-cli -p moq-relaycargo clippy -p moq-msf -p moq-mux -p moq-cli -p moq-relay --all-targets -- -D warningsRelease plz follow-up
This PR intentionally does not override versions in
Cargo.toml. Release-plz will detect the changes here (Track::newis genuinely breaking,Cluster::newarity changed) and propose new bumps:moq-msfwill likely go to0.2.0sinceTrack::newreplaces struct-literal construction; that's accurate.moq-relaywill still flag breaking changes (per the existing patch-bump-for-relay policy in #1467).The intent is that subsequent releases stop tripping the same lints, since the now-
#[non_exhaustive]types absorb additive changes.🤖 Generated with Claude Code