Repository navigation
quest: plan ffi-shape request accept and upgrade-page renames - #4701
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…accept Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
MERGE — quest/docs planning only; claims check out against the tree and #4697. Reviewed head: FindingsNo blocking issues. Non-blocking
Claims check
CI: Test passed; Check still pending (docs-only, not a concern). VerdictMERGE This is an automated review, not the maintainer's decision |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: ca44fe4
No actionable new bugs found in this planning-only change. Passing origins when accepting a request is a simpler FFI shape than separately mutating the request, and the revised explanation now accurately distinguishes Rust's consuming builders from the shared FFI handle (quest/m1/ffi-shape/request-accept.md:11–17). The upgrade-page additions match the changes in #4697.
One non-blocking clarification, independently confirming the existing review: state how optional accept arguments interact with the server's configured origins. #4697 still seeds each request from MoqServerConfig.publish/consume; the current setters can replace either with None. Preserve or explicitly decide omission/null/default behavior when implementing this quest, with coverage for inherited, overridden, and cleared origins. This is a planning clarification, not a demonstrated regression.
Verification: inspected both changed documents, the relevant Rust request/handshake code, and #4697 at 172d929. No local builds, tests, or quest checks run; the required tooling is unavailable. Check CI for the reviewed head was still running.
(Written by review (OpenAI))
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
MERGE — quest/docs planning only; Reviewed head: FindingsNo blocking issues. Non-blocking
What changed since
|
|
MERGE — quest/docs follow-up; Reviewed head: FindingsNo blocking issues. No remaining non-blocking issues. What changed since
|
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed delta: ca44fe4 → 22b24c7.
The previous clarification is addressed in quest/m1/ffi-shape/request-accept.md:18–22: server defaults remain, omission inherits them, and explicit clearing must be deliberately retained or removed with matching coverage. The remaining clearing choice is correctly left to implementation on the breaking-change branch.
No new actionable bugs found. The direction remains sound and keeps the API decision explicit without prescribing unnecessary machinery.
Verification: static review of the sole changed document and its surrounding plan. No builds, tests, or quest checks run.
(Written by review (OpenAI))
|
Merging. Adds (Written by Claude Opus 5.5) |
Follow-ups from #4697 (ffi-shape/net).
ffi-shape/request-accept[XS]:MoqRequestorigins becomeaccept()arguments, removing the last root setters.Decisions
Follow-ups from #4697 to plan
Where they go
Note:
quest checkfails on this line branch with errors that were already there before this PR (outside-conditionRequiredentries elsewhere in the tree); this PR adds none.Public API: none (planning). Wire: none.
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code