chore: clean up pre-existing comment noise - #42
Merged
Conversation
Same three patterns as the previous pass, in code that predates it. Zero-information doc comments. "SetLogger sets the structured logger", "SetDeviceResolver sets the device resolver", "SetUserIDGetter sets the function used to extract user ID" — none of them say anything the signature doesn't. Setters whose comment does carry something (the stream-token validator explaining the MPD fallback, SetBrowserRegistry explaining what changes when it's set) are left alone. Listing today's callers. SendToClient enumerated the client-id welcome and the transfer handoff; that list is wrong the moment a third caller appears, and the signature already says what the function does. History. PlayAlbumOnDevice explained itself in terms of "the bug that masked simultaneous browser+MPD playback" — the rule (a device that refuses the play must not be recorded as playing) is what a reader needs, and it stays true regardless of which bug prompted it. ServeWs called the hub-assigned counter "the old monotonic counter" while still using it as the live fallback, which reads as dead code that isn't. Deliberately not touched: the ~20 "NewX creates a new X" constructor comments. They're equally empty, but they're the house convention and Go's own doc style; removing five and leaving fifteen would be worse than leaving all of them. Worth a decision either way, repo-wide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
klabast
force-pushed
the
chore/comment-cleanup
branch
from
August 28, 2026 07:52
b11bd07 to
35a728a
Compare
klabast
added a commit
that referenced
this pull request
Aug 28, 2026
pull_request runs the commit stage only; merge_group runs the full pipeline against trunk + the queued change (the gate); push to main runs it again and promotes. Fixes three gaps found while merging #40/#41/#42: acceptance only ran after merge, nothing tested the prospective combination, and promote was an unguarded race on :latest where last writer wins. ADR in docs/adr/0001-trunk-based-delivery-pipeline.md.
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.
Stacked on #41 — review/merge that one first. Based on
fix/core-hardeningrather thanmainbecause #41 rewritesplayback/service.go,websocket/hub.goandauth/session.go, so amain-based branch would conflict on every hunk. Once #41 lands this retargets tomaincleanly.Follow-up to the comment pass in #41, applied to code that predates it. Same three patterns:
Zero-information doc comments.
SetLogger sets the structured logger,SetDeviceResolver sets the device resolver,SetUserIDGetter sets the function used to extract user ID— none say anything the signature doesn't. Setters whose comment does carry something (the stream-token validator explaining the MPD fallback,SetBrowserRegistryexplaining what changes when it's set) are untouched.Listing today's callers.
SendToClientenumerated the client-id welcome and the transfer handoff. That list is wrong the moment a third caller appears, and the signature already says what the function does.History.
PlayAlbumOnDeviceexplained itself in terms of "the bug that masked simultaneous browser+MPD playback". The rule — a device that refuses the play must not be recorded as playing — is what a reader actually needs, and it stays true regardless of which bug prompted it.ServeWscalled the hub-assigned counter "the old monotonic counter" while still using it as the live fallback, which reads as dead code that isn't.one thing I did not do, on purpose
There are ~20
// NewX creates a new Xconstructor comments. They're just as empty, but they're the house convention throughout the repo and they match Go's own doc style, sofollow-patterns.mdandcomments.mdpoint in opposite directions here. Removing five and leaving fifteen is worse than leaving all twenty — it's a repo-wide style call, and yours to make. Say the word and I'll do them all in one sweep.Comment-only; no behaviour change.
gofmt,go vet,go buildand the full suite green.🤖 Generated with Claude Code