Skip to content

moq-clock: convert to a moq-native example#1494

Merged
kixelated merged 1 commit into
mainfrom
claude/romantic-mcnulty-6f3ff2
May 25, 2026
Merged

moq-clock: convert to a moq-native example#1494
kixelated merged 1 commit into
mainfrom
claude/romantic-mcnulty-6f3ff2

Conversation

@kixelated

@kixelated kixelated commented May 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Delete the standalone rs/moq-clock crate and move its source into rs/moq-native/examples/clock.rs, alongside the existing chat.rs example.
  • Drop moq-clock from the Cargo workspace, the Nix overlay/flake, .release-plz.toml, and the demo/pub/justfile.
  • Update docs (CLAUDE.md, rs/moq-net/README.md, doc/lib/rs/index.md, js/clock/README.md, py/moq-rs/examples/clock.py) to point at the new location.

Run with:

cargo run -p moq-native --example clock -- --url https://relay.example.com/anon --broadcast clock publish
cargo run -p moq-native --example clock -- --url https://relay.example.com/anon --broadcast clock subscribe

Why

moq-clock was already marked publish = false after #1488 and had no external consumers. Keeping it as a workspace member just added another binary to build, a Nix derivation, and a release-plz config block that we had to maintain. As an example it stays buildable alongside the workspace and lives next to the existing chat example that the moq-net README already links to.

This also fixes the release-plz release failure on main (https://github.com/moq-dev/moq/actions/runs/26375450178/job/77634868258). With the crate gone, the publish = false (Cargo.toml) vs publish = true (release-plz default) mismatch no longer exists, so the .release-plz.toml package block is dropped entirely instead of being patched.

Placement note

Examples that demonstrate moq-net usage with moq-native helpers (logging, ClientConfig, etc.) already live in rs/moq-native/examples/ (see chat.rs). Putting clock there too avoids a dev-dep cycle (moq-net → moq-native → moq-net) and matches that precedent. The moq-net README continues to link to both examples.

Test plan

  • cargo check --all-targets (default members) passes
  • cargo build -p moq-native --example clock succeeds
  • cargo run -p moq-native --example clock -- --help shows the expected CLI
  • just rs check (check, clippy, fmt, doc, shear, sort) passes
  • After merge: next Release RS run on main reaches release-plz release without the publish-mismatch error.

🤖 Generated with Claude Code

(Written by Claude)

@kixelated kixelated enabled auto-merge (squash) May 24, 2026 23:32
@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR removes the rs/moq-clock crate and its ancillary files from workspace and packaging outputs, updates documentation and README links to point to a new clock example added at rs/moq-native/examples/clock.rs, changes the demo justfile to run that example, adds chrono to rs/moq-native dev-dependencies, and updates the Nix overlay to remove moq-clock and introduce a moq-relay package.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: converting the moq-clock crate from a standalone workspace member into an example within moq-native.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the rationale, implementation details, run instructions, and test plan for converting moq-clock to a moq-native example.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/romantic-mcnulty-6f3ff2

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kixelated kixelated disabled auto-merge May 24, 2026 23:32
Replaces the standalone moq-clock crate with rs/moq-native/examples/clock.rs,
joining the existing chat example. The clock binary was already marked
publish = false on crates.io and had no external consumers; folding it
into the examples folder keeps it building with the workspace, drops a
member from `cargo check`'s target set, and removes the release-plz
config and Nix derivation that existed only to build a binary nobody
shipped.

Also fixes the release-plz CI failure on main: with the crate gone, the
`publish = false` (Cargo.toml) vs `publish = true` (release-plz default)
mismatch that aborted `release-plz release` no longer exists, so the
`.release-plz.toml` package block is dropped entirely.

Run with `cargo run -p moq-native --example clock -- --url ... --broadcast clock {publish,subscribe}`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kixelated kixelated force-pushed the claude/romantic-mcnulty-6f3ff2 branch from 2c77dd3 to 0dacb88 Compare May 24, 2026 23:41
@kixelated kixelated changed the title release-plz: set publish = false on moq-clock moq-clock: convert to a moq-native example May 24, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@rs/moq-native/examples/clock.rs`:
- Around line 138-141: Replace the panic-worthy .unwrap() on create_group with
the ? operator so errors are propagated from run(): change the line using
self.track.create_group(sequence.into()).unwrap() to use
self.track.create_group(sequence.into())? and keep the rest of the loop intact;
ensure the surrounding run() signature returns a compatible Result or map the
error (e.g., using .map_err(...)? ) if the error types differ so compilation
succeeds.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e3f37277-fc9f-41d9-bc42-b29f6f310aee

📥 Commits

Reviewing files that changed from the base of the PR and between 2c77dd3 and 0dacb88.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (18)
  • .release-plz.toml
  • CLAUDE.md
  • Cargo.toml
  • demo/pub/justfile
  • doc/lib/rs/index.md
  • flake.nix
  • js/clock/README.md
  • nix/overlay.nix
  • py/moq-rs/examples/clock.py
  • rs/moq-clock/CHANGELOG.md
  • rs/moq-clock/Cargo.toml
  • rs/moq-clock/README.md
  • rs/moq-clock/build.rs
  • rs/moq-clock/src/clock.rs
  • rs/moq-clock/src/main.rs
  • rs/moq-native/Cargo.toml
  • rs/moq-native/examples/clock.rs
  • rs/moq-net/README.md
💤 Files with no reviewable changes (11)
  • rs/moq-clock/README.md
  • rs/moq-clock/src/main.rs
  • nix/overlay.nix
  • Cargo.toml
  • rs/moq-clock/build.rs
  • doc/lib/rs/index.md
  • rs/moq-clock/CHANGELOG.md
  • rs/moq-clock/src/clock.rs
  • flake.nix
  • .release-plz.toml
  • rs/moq-clock/Cargo.toml
✅ Files skipped from review due to trivial changes (4)
  • rs/moq-net/README.md
  • js/clock/README.md
  • CLAUDE.md
  • py/moq-rs/examples/clock.py

Comment on lines +138 to +141
loop {
let segment = self.track.create_group(sequence.into()).unwrap();

sequence += 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Prefer ? over .unwrap() for create_group.

If create_group fails, this will panic. Since run() returns Result, propagate the error instead.

Suggested fix
-			let segment = self.track.create_group(sequence.into()).unwrap();
+			let segment = self.track.create_group(sequence.into())?;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
loop {
let segment = self.track.create_group(sequence.into()).unwrap();
sequence += 1;
loop {
let segment = self.track.create_group(sequence.into())?;
sequence += 1;
🤖 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-native/examples/clock.rs` around lines 138 - 141, Replace the
panic-worthy .unwrap() on create_group with the ? operator so errors are
propagated from run(): change the line using
self.track.create_group(sequence.into()).unwrap() to use
self.track.create_group(sequence.into())? and keep the rest of the loop intact;
ensure the surrounding run() signature returns a compatible Result or map the
error (e.g., using .map_err(...)? ) if the error types differ so compilation
succeeds.

@kixelated kixelated enabled auto-merge (squash) May 24, 2026 23:50
@kixelated kixelated merged commit 7489ae6 into main May 25, 2026
1 check passed
@kixelated kixelated deleted the claude/romantic-mcnulty-6f3ff2 branch May 25, 2026 00:02
@moq-bot moq-bot Bot mentioned this pull request May 25, 2026
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.

1 participant