Repository navigation
refactor(rs)!: ban doc(hidden); document, delete, or privatize each use - #8392
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Quest-Claim: 6c7a11b7f3ca9b94
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
|
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 24 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. |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: e0c2c12
[P2] Match hidden after other doc options (sh/text.sh:37-38). Rust accepts #[doc(alias = "OldName", hidden)] pub struct Example;, but this pattern only recognizes hidden immediately after doc(. A tracked Rust file containing that attribute therefore passes the new scan while rustdoc still hides the public item, including if it arrives in vendored code. Check all options in a doc attribute, not only the first, and add this case to sh/text.test.py.
Direction: keeping test support feature-gated and consolidating QMux at connection::mux fits the recorded plan; the enforcement gap above should be closed before treating the ban as complete.
Limits: GitHub-only static review of the diff, gate/tests, and selected API context. Rust's doc-attribute parser confirms it processes every option in the list. I did not run the regression example, quest context, Rust builds, or tests.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The new gate misses a valid doc attribute form: #[doc(alias = "OldName", hidden)] passes sh/text.sh while rustdoc still hides the item, so the ban is bypassable. The fix is a one-line pattern change plus a test case.
Reviewed changes
doc(hidden)gate insh/text.sh:check:textnow rejectsdoc(hidden)in any tracked*.rs(vendored included), includingcfg_attrand spacing variants, sharing aforbidhelper with the em-dash scan so one run reports both.sh/text.test.pygains coverage for the new cases.- Vendored attributes deleted: every
#[doc(hidden)]inrs/moq-webrtc/src/vendoris gone; those modules are private, so nothing was hidden. - Feature-gated test support documented:
fuzzin moq-net, moq-quic, moq-mux, and moq-rtmp;simin moq-quic, moq-tokio, andquic::driver::sim; plusmoq_json::bench,moq_stats::produce::bench, andmoq_quic::connection::BenchSentPackets. moq_tokio::settingsandmoq_webrtc::p2pdocumented as public API.- Breaking
muxmove: themoq_quic::muxroot re-export is removed andqmuximportsmoq_quic::connection::mux; theMIGRATE.mdentry is rewritten. - Wording sweep: "hidden module" comments, READMEs, Cargo feature notes,
bench/README.md, and quest docs updated; the finished quest file is deleted.
ℹ️ One stale "hidden" reference survives the sweep
.github/relay-cpu-bisect.md:201 still describes fuzz::ObjectBatch as "the hidden fuzz::ObjectBatch helper", while the identical sentence in bench/README.md:447 was updated to fuzz::ObjectBatch. Since the module is documented test support now, the adjective is stale in one of its two homes.
Technical details
# Stale `hidden` wording in the bench report
## Affected sites
- `.github/relay-cpu-bisect.md:201` — "through the hidden `fuzz::ObjectBatch` helper"
- `bench/README.md:447` — the same sentence, already updated
## Required outcome
- Both sites describe `fuzz::ObjectBatch` the same way.deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
…ange Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: e48e10a
Incremental review since: e0c2c12
Accounted for the main merge to ac3cb8c; the substantive follow-up is the four-file fix in f8731b7.
The prior P2 is addressed: sh/text.sh:38 now matches hidden after preceding doc options, and sh/text.test.py:90 includes the reported #[doc(alias = "Old", hidden)] case. Both moq-tokio sim modules now explicitly describe their test-support stability policy.
No new actionable findings in this delta. Direction remains consistent with the recorded plan: keep test support feature-gated and documented, and enforce the ban centrally.
Limits: GitHub-only static review of the follow-up files and base-relative changes. I did not execute the gate/regression tests, Rust builds, or quest checks. This is not a fresh audit of the unchanged portions of the original PR. COMMENT only, not approval.
There was a problem hiding this comment.
ℹ️ No critical issues — the gate fix and sim docs land cleanly; one stale "Hidden from the docs" note is left inline.
Reviewed changes
doc(hidden)gate closed: the pattern is nowdoc[[:space:]]*\([^)]*hidden, so#[doc(alias = "Old", hidden)]failssh/text.sh, andsh/text.test.pycovers that form.moq-tokiosim docs completed: bothsimandquic::driver::simnow end with "It tracks the crate's internals, so it can change in any release", andsimexplains that the handshake, redials, and GOAWAY handling still run as over a socket.origin/mainmerged atac3cb8c8; no branch-side changes beyond the above.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
| pub mod settings; | ||
| #[cfg(all(feature = "sim", feature = "net"))] | ||
| #[doc(hidden)] | ||
| pub mod sim; |
There was a problem hiding this comment.
sim is documented now, but rs/moq-tokio/Cargo.toml:54 still closes the sim feature note with "Hidden from the
docs." That is the one Cargo feature comment the sweep missed; moq-quic, moq-rtmp, moq-mux, moq-json, and moq-stats all dropped that clause in this PR.
Finishes quest/m2/api-shape/doc-hidden.md.
Problem
#[doc(hidden)]left public Rust items in the API with no documentation, so callers could depend on them without knowing what they were. The maintainer asked to ban it after moq-dev/moq#8369, which removed the hides on moq-net's route stream.Approach
sh/text.sh(just check'scheck:text, which runs on every branch) now rejectsdoc(hidden)in any tracked*.rsfile, vendored code included. It also matchescfg_attr(..., doc(hidden))and spacing variants. The em dash scan and the new scan share oneforbidhelper, and both are reported in a single run.sh/text.test.pycovers it.rs/moq-webrtc/src/vendor, 34 attributes): deleted. Those modules are private, so the attribute hid nothing.pubbehind its feature, and its doc line now says it is test support and names the feature:fuzzin moq-net, moq-quic, moq-mux, and moq-rtmp;simin moq-quic, moq-tokio, andmoq_tokio::quic::driver::sim;moq_json::bench;moq_stats::produce::bench; andmoq_quic::connection::BenchSentPackets. Each module doc also says it tracks the crate's internals and can change in any release. Items thatmissing_docsnow reaches (fuzz::CanonicalStorage::{new, size}andsim::Bottleneck::stats) are documented.moq_tokio::settings: documented public API, because moq-relay and moq-bench read it.moq_webrtc::p2p: documented public API, becausemoq-cli --p2p(quest/m4/p2p/cli.md) is a planned consumer.mux: now reachable only atmoq_quic::connection::mux. The duplicatemoq_quic::muxre-export is gone, and qmux imports the module fromconnection::mux.Impact
moq_quic::muxre-export is removed. Usemoq_quic::connection::muxinstead (noted inrs/moq-quic/MIGRATE.mdunder Unreleased).moq_tokio::settingsandmoq_webrtc::p2p(featurep2p), plus the feature-gated test support listed above, in moq-net, moq-quic, moq-mux, moq-rtmp, moq-json, moq-stats, and moq-tokio. Default-feature builds expose no new items.Choices made without asking
connection::muxand dropped the rootmuxre-export instead of the other way round. The module lives underconnectionand borrows that module's internals, and keeping only one path means no#[path]workaround.Alternatives
Validation
just checkpasses every module on Linux, includingcheck rs,test rs,test gh, andcheck text.bash sh/text.shpasses, andpython3 sh/text.test.pypasses (8 tests).cargo clippy --all-targets --all-features -- -D warningspasses on moq-net, moq-quic, moq-mux, moq-rtmp, moq-json, moq-stats, moq-tokio, moq-webrtc, qmux, moq-relay, and moq-net-tests.RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-featurespasses on moq-net, moq-quic, moq-mux, moq-rtmp, moq-json, moq-stats, moq-tokio, moq-webrtc, and qmux.quest check: ok.just check: every module passes exceptcheck rsandtest gh, and both of those fail the same way onorigin/main. Withbridge(moq-carla-bridge) left out, I reran the remainingcheck rslint steps:cargo doc -D warningsover every selected crate,cargo shear,cargo sort --check, andcargo hakari generate --diff. They all pass.origin/maincheckout (7e50202) on this macOS host, so this PR did not cause them:test gh:python3 sh/version.test.pyfailstest_cut_promotes_only_published_library_dependencies_and_preserves_exemptions('0.0.1' != '0.22.0') andtest_release_plz_keeps_manual_cut_despite_breaking_commits, with release-plz 0.3.149.check rs:RUSTDOCFLAGS="-D warnings" cargo doc --no-deps -p moq-carla-bridgereports unresolved links toBridge,Source, andprotocolindemo/carla/bridge/src/lib.rs.cargo clippy -p moq-bench --all-targets --all-featuresreports an unfulfilledexpect(dead_code)atrs/moq-bench/src/host.rs:43. This PR does not touch that file.just ci. Only visibility and docs changed, and every affected crate compiles with--all-targets --all-features.Follow-ups
sh/version.test.pyfails locally onmainwith release-plz 0.3.149, andmoq-carla-bridge's docs fail-D warningson macOS (see Validation). No open PR fixes either one yet.Models
Size: M
Implementer: Claude Code, claude-opus-5-5, unknown effort
Implementer: Claude Code, claude-opus-5-5, medium effort (iterate, M)
Review feedback
sh/text.shnow matcheshiddenanywhere insidedoc(...), so#[doc(alias = "Old", hidden)]fails too;sh/text.test.pycovers it.simmodules in moq-tokio now say they track the crate's internals and can change in any release, like theirfuzzsiblings.Review
Rewritten by
quest reviewafter each round.01a1c38d-23cb-7a21-bed1-5024db1bb9c7, round 1The documentation/API cleanup and text-check changes appear sound from source inspection, with no actionable defects found; builds and tests were not run, as instructed.