refactor(net): finish the Hop rename in test names and log fields - #3264
Merged
Conversation
The Hop rename left the identifier's old spelling in a few internal identifiers: seven test functions still said peer_origin, the cluster startup log emitted an origin_id field, and two locals held a bare Hop under the name origin. None of these are published surface, so they were left out of the rename that landed them. The log field is the one a reader could trip on: every other hop id the relay reports, including GET /nodes, now spells it hop_id. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Follow-up to #3252, which renamed the hop identifier from
OrigintoHop. That PR was scoped to the published surface and deliberately stopped there, so a few internal identifiers kept the old spelling.peer_origin, sitting directly under thewith_peer_hop/peer_hopAPI they exercise (moq-net:server.rs,lite/subscriber.rs,ietf/subscriber.rs,ietf/publisher.rs).origin_id(moq-relay/src/cluster.rs). This is the one a reader could actually trip on: the value isorigin.id(), aHop, and every other hop id the relay reports now spells ithop_id— includingGET /nodes, which refactor(net)!: name the hop identifier Hop, and fix three API shapes the review found #3252 renamed. The two disagreeing was a live inconsistency in operator-facing output, not just naming taste.Hopunder the nameorigin(server.rs,tests/goaway.rs). Note that mostoriginlocals in these files are correct and untouched: they hold anorigin::ProducerfromHop::…().produce(), which is genuinely the routing table.Also unchanged, for the same reason #3252 gave:
origin::Producer's privateinfofield still holds aHop, and untangling it collides with a dozen sites whereinfois a local for the realInfo.No public surface moves, so this targets
devonly because the rename it finishes lives there.Verification
just checkpassed but dispatched no Rust step at all despite a diff of sixrs/files, so the Rust half was run directly against the same nix-store toolchain instead:cargo clippy -p moq-net -p moq-relay --all-targets --locked -- -D warnings— cleancargo nextest run -p moq-net -p moq-relay --locked— 1297 passed, 1 skipped-E "test(peer_hop)"), so the rename didn't quietly orphan themThat
just checkskipped Rust here looks like a real bug in the diff-aware scoping rather than anything about this branch, and it's worth a look on its own — it means a Rust-only change can passjust checklocally without being compiled. Filing separately.(Written by Claude Opus 5)