You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(moq-video): nightly's Windows platform job passes - #8429
Finishes quest/m0/nightly/windows.md. platform.yml's Windows job fails on main because Backend::encode_all, a test helper from moq-dev/moq#8372, is never used in the render-only test build (just rs test -p moq-video --no-default-features --features render), and build.warnings = deny turns that into an error. It now compiles only where a backend test calls it: OpenH264, Apple, Android MediaCodec, Linux VA-API.
Of the quest's three known causes, only this one remained on main:
Local Linux clippy did not run: quest gate waited an hour on host load without starting.
Choices made without asking
Kept flush_all's existing #[allow(dead_code)]: gating it the same way needs a second, different backend list (OpenH264's tests don't call it), which drifts more than it saves.
Caused by
moq-dev/moq#8372 - added encode_all as plain #[cfg(test)] with no caller on Windows.
Public API: none. Wire: none.
Models
Size: S
Implementer: Claude Code, claude-opus-5-5, low effort
Score: 9/10, The conditional compilation matches the backend tests using the helper and preserves production behavior; source review found no actionable defects, with builds and tests intentionally not run.
…t calls it
Windows' render-only test build has no backend that calls it, so build.warnings
= deny failed platform.yml's Rust tests.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
Nightly's Windows platform job passes
fix(moq-video): nightly's Windows platform job passes
Nov 10, 2026
No actionable defects found. The test-only encode_all helper's cfg matches its backend-test callers (OpenH264, VideoToolbox, Android MediaCodec, and Linux VA-API), removing it from Windows render-only builds without changing production behavior. The completed quest and its README entry are removed together. Overall direction: the targeted conditional-compilation fix looks sound.
Verification: inspected the full diff and backend call sites through GitHub. The linked after-run 42801241203 reports successful Windows and macOS jobs, including Rust compile/tests. I did not run local builds/tests or independently exercise every platform/feature combination.
The reason will be displayed to describe this comment to others. Learn more.
✅ No new issues found.
Reviewed changes
Gated Backend::encode_all to its real callers — the test helper is now compiled only under test plus one of openh264, apple, Android mediacodec, or Linux vaapi, so the render-only Windows test build no longer trips build.warnings = deny with dead_code. flush_all keeps its #[cfg(test)] + allow(dead_code) for the reason the body records.
Completed the quest — deleted quest/m0/nightly/windows.md and its ## Required bullet in quest/m0/nightly/README.md; no other file referenced the slug.
I audited every two-argument encode_all call site in the crate: they are all in the vaapi, openh264, mediacodec, and videotoolbox test modules, and each module's own cfg is one of the four in the new gate. nvenc, v4l2, and mediafoundation tests only call the encode::Encoder helper (one argument), so they compile unchanged where the new gate is false — including the just rs media-features--features nvidia and bare --no-default-features shapes. Default features are mediacodec, nvidia, vaapi, so the gate is false on Windows (the failing build) and true on Linux, macOS, and Android as intended. Run 42801241203 on this branch confirms Windows Rust compile and Rust tests both pass.
The hand-maintained cfg list duplicates the backend module gates, but drift in either direction fails loudly (unused helper → dead_code, removed helper → compile error), so it needs no extra guard.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: c447977e-5aab-45e8-af3c-c537f7ee9868
Commits
Reviewing files that changed from the base of the PR and between 8f0fee8 and 22b2ba4.
Files selected for processing: 3
quest/m0/nightly/README.md
quest/m0/nightly/windows.md
rs/moq-video/src/encode/backend/mod.rs
No actionable comments were generated in the recent review.
Walkthrough
The test-only encode_all helper now compiles only when the build enables OpenH264, an Apple target, Android with mediacodec, or Linux with vaapi. The changes also remove the Nightly Windows README item and the document describing the Windows job plan.
Merge Risk: Minimal · previous review 8f0fe
The helper remains available to the inspected encoder tests. No actionable merge-blocking risk was identified.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title clearly identifies the Windows nightly job fix and matches the main change: gating Backend::encode_all to avoid an unused-code error.
Description check
The description explains the Windows test failure, the conditional compilation fix, and the reported verification. It is directly related to the changeset.
Docstring Coverage
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.
Finishing Touches
Simplify code
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: b65152a1-e751-4f94-88e6-038bcd1896cc
Commits
Reviewing files that changed from the base of the PR and between cd560bd and 8f0fee8.
Files selected for processing: 3
quest/m0/nightly/README.md
quest/m0/nightly/windows.md
rs/moq-video/src/encode/backend/mod.rs
Files with no reviewable changes: 2
quest/m0/nightly/windows.md
quest/m0/nightly/README.md
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
quest land watch: the merge queue removed this pull request at 8f0fee8 because it conflicts with main (merge_conflict).
The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.
The reason will be displayed to describe this comment to others. Learn more.
Caution
The new merge commit 74d26316c deletes the child list from every quest README, so merging this PR would remove 39 ## Required sections and 668 lines that main still has.
Reviewed changes
Re-reviewed the delta since the prior pullfrog review (8f0fee8).
Merged main at 5e962fc — merge commit 74d26316c brings in #8427 and #8397. The Backend::encode_all gate and the quest/m0/nightly/windows.md deletion from 8f0fee8 are unchanged.
The merge resolution regressed quest content — the resulting tree no longer matches main for 43 quest READMEs (below).
🚨 The merge dropped every quest README's child list
git diff origin/main...HEAD is now 45 files with 668 deletions, and almost all of it is quest content that main still has. 39 quest READMEs lose their ## Required section (quest/README.md, every m0–m5 index, and the epic READMEs), quest/m3/mesh/README.md also loses its ## Related child bullets, and quest/m5/e2ee/README.md loses its own child list. quest/m0/nightly/README.md loses the macOS bullet too, not just the intended Windows one.
This is not part of the fix: the branch at 8f0fee8 and main at 5e962fc both carry these lists, so the "merge tested base" step produced content neither parent had. The Quest check passes without them, so it will not catch this before it lands.
Technical details
# Restore the quest lists the merge dropped## Affected sites-`quest/README.md` — `## Required` deleted (old lines 55-62); the milestone index.
-`quest/m0/README.md`, `quest/m1/README.md`, `quest/m2/README.md`, `quest/m3/README.md`, `quest/m4/README.md`, `quest/m5/README.md` — same.
-`quest/m0/ci/README.md`, `quest/m0/nightly/README.md`, `quest/m0/test-flakes-2/README.md` — same.
-`quest/m1/*/README.md` (3), `quest/m2/*/README.md` (9), `quest/m3/*/README.md` (14), `quest/m4/*/README.md` (4), `quest/m5/e2ee/README.md`, `quest/m5/net-wrap/README.md` — same.
-`quest/m3/mesh/README.md` — the `## Related` child bullets are gone while the heading remains.
-`quest/m0/nightly/README.md` — the macOS bullet was dropped along with the Windows one it was meant to remove.
## Required outcome- The quest files match `main` except for the intended removal of `quest/m0/nightly/windows.md` and its own bullet in `quest/m0/nightly/README.md`.
- Re-merge `main` (or rebase the two branch commits) so the "quest land: merge tested base" commit no longer reverts main's quest content.
## Suggested approach- Re-run the merge and confirm with `git diff origin/main...HEAD --stat` that only `rs/moq-video/src/encode/backend/mod.rs`, `quest/m0/nightly/windows.md`, and `quest/m0/nightly/README.md` differ.
-`git diff origin/main...HEAD -- quest/ | grep '^-##'` should then show only the `windows.md` headings.
## Open questions for the human- Is the quest-land merge driver expected to rewrite these lists? If it regenerated them from a stale tree, the driver likely needs the fix upstream too.
quest land watch: the merge queue removed this pull request at 74d2631 because it conflicts with main (merge_conflict).
The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.
The reason will be displayed to describe this comment to others. Learn more.
✅ No new issues found.
Reviewed changes
Re-reviewed the delta since the prior pullfrog review (74d2631).
Restored the quest content the stale merge dropped — 22b2ba4 re-adds the 370 lines of ## Required / ## Related child lists across 38 quest READMEs, including the quest/m0/nightly/README.md macOS bullet, so the branch no longer reverts main's quest tree.
The fix and the quest completion are untouched — the Backend::encode_all gate and the quest/m0/nightly/windows.md deletion are unchanged from 8f0fee8.
The branch now differs from main in exactly the three intended files (+9 / -33), and nothing references /quest/m0/nightly/windows.md anymore. The prior request-changes (review 5845171820) is fully addressed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
kixelated
changed the title
kixelated
left a comment
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Summary
Finishes
quest/m0/nightly/windows.md.platform.yml's Windows job fails on main becauseBackend::encode_all, a test helper from moq-dev/moq#8372, is never used in the render-only test build (just rs test -p moq-video --no-default-features --features render), andbuild.warnings = denyturns that into an error. It now compiles only where a backend test calls it: OpenH264, Apple, Android MediaCodec, Linux VA-API.Of the quest's three known causes, only this one remained on main:
moq-vaapioff Linux: already fixed by moq-dev/moq#8258.moq-relaymemory test unused items: already fixed; the 2026-11-09 nightly'splatform / Windowsran themoq-relaytests green.encode/backend/mod.rsdead code: on main since feat(video)!: codecs split submission from receive #8372, which merged after that green nightly. Fixed here.Verification
quest gatewaited an hour on host load without starting.Choices made without asking
flush_all's existing#[allow(dead_code)]: gating it the same way needs a second, different backend list (OpenH264's tests don't call it), which drifts more than it saves.Caused by
encode_allas plain#[cfg(test)]with no caller on Windows.Public API: none. Wire: none.
Models
Size: S
Implementer: Claude Code, claude-opus-5-5, low effort
🤖 Generated with Claude Code
(Written by Claude Opus 5.5 with low)
Review
mechanical fix by quest land watch:
merged main at 5e962fc7f1f559f79212a1920fe12b0f6b76bb39 with the quest merge driver(head 74d2631)Rewritten by
quest reviewafter each round.01a1c530-8a4d-7c73-b5ca-19597716625c, round 1The conditional compilation matches the backend tests using the helper and preserves production behavior; source review found no actionable defects, with builds and tests intentionally not run.