Bring comments up to idiomatic Go doc-comment standards#26
Open
jclusso wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR standardizes and trims comments across the Go codebase to better match idiomatic Go doc-comment conventions (name-first comments on exported identifiers, less narration on internal code) and removes the “comments are a last resort” guidance from AGENTS.md.
Changes:
- Rewrites/condenses package and exported-identifier doc comments across
cmd/andinternal/packages. - Reduces internal/unexported commentary to “why/gotcha/spec” style notes.
- Updates contributor guidance by removing the “comments are a last resort” rule in
AGENTS.md.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/embed.go | Adds an exported doc comment for the embedded FS. |
| internal/updater/updater.go | Trims and normalizes updater package/type/function comments. |
| internal/ui/theme.go | Refines doc comments for exported theme/keymap helpers. |
| internal/ui/style.go | Shortens doc comments for ANSI styling helpers. |
| internal/ui/spinner.go | Condenses package/type/function comments for TTY/spinner utilities. |
| internal/ui/select.go | Updates exported type/function comments for selection prompt. |
| internal/ui/prompt.go | Updates exported function comments for input prompting. |
| internal/ui/confirm.go | Updates exported function comment for confirm prompt. |
| internal/ui/brand.go | Trims internal/external comments around brand rendering assets. |
| internal/ui/brand_anim.go | Condenses animation-related commentary while keeping key “why” notes. |
| internal/ui/bar.go | Shortens comments for progress bar behavior and API. |
| internal/skill/skill.go | Adds/normalizes exported identifier comments for skill installation helpers. |
| internal/output/output.go | Condenses package-level and constructor doc comments. |
| internal/output/json.go | Shortens JSON formatter and raw passthrough documentation. |
| internal/output/jq.go | Updates comments for jq query compilation and execution wrapper. |
| internal/output/human.go | Trims/standardizes human formatter comments. |
| internal/output/file.go | Simplifies comments for file output options and format selection. |
| internal/output/account_view.go | Condenses struct documentation while preserving rationale. |
| internal/oauth/oauth.go | Normalizes OAuth client and exported error/type comments. |
| internal/env/projcfg.go | Trims project-config discovery and parsing commentary. |
| internal/env/env.go | Condenses env resolution docs and exported constants/type comments. |
| internal/credentials/credentials.go | Simplifies credentials schema/path/load/save doc comments. |
| internal/config/config.go | Condenses config package docs and exported type/function comments. |
| internal/api/types.go | Shortens API type comments while preserving payload-shape notes. |
| internal/api/errors.go | Simplifies API error and rate limit doc comments. |
| internal/api/client.go | Condenses client/options docs; keeps key retry behavior note inline. |
| cmd/welcome.go | Trims onboarding/welcome flow narration into terser comments. |
| cmd/version.go | Removes verbose command/function comment blocks. |
| cmd/verify.go | Condenses flag-to-options rationale comment. |
| cmd/status.go | Removes verbose command/renderer comment blocks. |
| cmd/skill.go | Removes verbose command-group comment. |
| cmd/root.go | Trims root command and help/version/update-notifier commentary. |
| cmd/man.go | Removes verbose manpage-generation comment block. |
| cmd/logout.go | Removes verbose logout flow comments. |
| cmd/login.go | Condenses rationale comments around auth flows and persistence. |
| cmd/errors.go | Trims detailed error taxonomy/rendering commentary. |
| cmd/email_inputs.go | Condenses CLI email/file input parsing rationale comments. |
| cmd/context.go | Trims cmd context/auth resolution commentary; keeps key inline note. |
| cmd/batch.go | Condenses polling/progress/streaming commentary. |
| cmd/account.go | Removes verbose command-group comment. |
| AGENTS.md | Removes “comments are a last resort” guideline. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Document every exported type, func, method, const, and var with a terse, name-first doc comment, and keep a package comment on each package — matching the Go standard library and common Go practice (e.g. basecamp-cli). Trim the narration and multi-line rationale that had accumulated on internal and unexported code, keeping only comments that explain a non-obvious why: a gotcha, trade-off, or spec quirk. Drop the "comments are a last resort" rule from AGENTS.md; the committed code now sets the convention. Comments-only change — no code, logic, or output strings were modified. golangci-lint (default config, and revive's exported rule), go vet, build, and the full test suite all pass.
jclusso
force-pushed
the
idiomatic-go-comments
branch
from
18cef1b to
115e3bf
Compare
Consolidate the duplicated PrintVerifyResult doc comment into one block, and reapply the comment trims that didn't survive the rebase onto master — the conflict resolution had kept master's verbose comments where the comment branch overlapped master's retry/auth refactors (waitForCompletion, requireAuth, renderError, do, backoffFor, parseRateLimit, the retry-knob consts, and the RateLimit type), so restore the terse versions while keeping master's code. The empty // line above the go:embed directive in skills/embed.go is left as-is: gofmt requires that separator between a doc comment and a //go: directive.
jclusso
force-pushed
the
idiomatic-go-comments
branch
from
115e3bf to
1de09c1
Compare
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
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.
Summary
AGENTS.md; the committed code now sets the convention.Why
The codebase had drifted to ~22% comment-only lines — explanatory prose on nearly every declaration, much of it restating signatures. That's both noisy and contrary to idiomatic Go, which documents exported identifiers but keeps internal comments sparse. This brings it in line with the standard library and comparable Go CLIs (e.g.
basecamp-cli).Result
reviveexported: 0 findings); comment density down to ~5.4%.brandconstants regrouped onto adjacent lines once their doc comments were removed.golangci-lint(default config),go vet,make build, and the full test suite pass.Test plan
make buildgo test ./...go vet ./...golangci-lint run ./...