chore: remove decorative comments for minimal style#28
Merged
Conversation
- Remove all decorative section banners (====, ----, ───) - Remove all phase headers (// Phase X:) - Remove redundant comments that repeat test/function names - Keep only comments that explain non-obvious context, safety rationale, or constraints - Update AGENTS.md with new 'Comments must be minimal' rule (rule #10) - Improves code readability by reducing noise in test files and implementation Test coverage: 406 passing with race detection, lint clean
|
✅ All checks passed (Go 1.26) Coverage: 68.4% |
There was a problem hiding this comment.
Pull request overview
This PR standardizes comment style across the codebase by removing decorative/duplicative comment banners (especially in tests) and documenting the new minimal-comment rule in AGENTS.md, aiming to improve signal-to-noise and keep only explanatory rationale.
Changes:
- Removed decorative section banners and phase headers across e2e/unit tests and some CLI implementation files.
- Removed redundant comments that simply restate test/function intent.
- Added
AGENTS.mdLandmine #10 defining the new minimal, non-decorative comment policy.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/runner/parallel_test.go | Removed decorative test section banners/redundant verification comments. |
| internal/git/git_test.go | Removed decorative section banners and some inline explanatory comments in git integration tests. |
| internal/git/git_more_test.go | Removed decorative banners and some scenario setup comments in extended git tests. |
| internal/e2e/update_test.go | Removed phase headers and “should …” comments that restate assertions. |
| internal/e2e/track_test.go | Removed decorative phase banner; kept TUI limitation context. |
| internal/e2e/status_test.go | Removed phase header and decorative helper separators. |
| internal/e2e/repo_test.go | Removed phase header and redundant “verify …” comments in repo e2e flows. |
| internal/e2e/interactive_test.go | Removed phase banners while retaining non-TTY/TUI constraints via notes/skips. |
| internal/e2e/helpers_test.go | Removed decorative separators and a redundant build comment in shared e2e helpers. |
| internal/e2e/help_test.go | Removed phase headers and redundant “should …” comments for help/version/unknown cmd tests. |
| internal/e2e/checkout_test.go | Removed phase header and redundant branch expectation comments. |
| internal/e2e/branch_test.go | Removed phase header and redundant “verify …” comments in branch e2e tests. |
| internal/db/db_test.go | Removed decorative banners and redundant comments in DB unit tests. |
| internal/config/config_test.go | Removed redundant “verify …” comments that restate assertions. |
| internal/cli/upgrade_test.go | Removed some explanatory headers around HTTP download boundary tests. |
| internal/cli/upgrade_bundle_compat_test.go | Removed redundant “verify …” comments while retaining in-test rationale where present. |
| internal/cli/update_test.go | Removed redundant test doc comments that restate test names. |
| internal/cli/update_run_test.go | Removed redundant “Finding #…” and setup comments in update runner tests. |
| internal/cli/status_test.go | Removed redundant test doc comments that restate test names. |
| internal/cli/stash_test.go | Removed redundant test doc comments and verification headings. |
| internal/cli/stash.go | Removed decorative section header comments between subcommand implementations. |
| internal/cli/root_test.go | Removed redundant test doc comments that restate test names. |
| internal/cli/reset_test.go | Removed decorative section banners between test groups. |
| internal/cli/reset.go | Removed “Phase X” header comments within runResetWithUI. |
| internal/cli/repo_test.go | Removed decorative banners and redundant preface comments in repo CLI tests. |
| internal/cli/discard_test.go | Removed redundant test doc comments that restate test names. |
| internal/cli/discard_run_test.go | Removed redundant scenario comments that restate code/test intent. |
| internal/cli/commit_test.go | Removed redundant test doc comments that restate test names. |
| internal/cli/commit_run_test.go | Removed redundant “should not error …” comments that restate assertions. |
| internal/cli/checkout_test.go | Removed redundant test doc comments that restate test names. |
| internal/cli/checkout_run_test.go | Removed redundant “Finding #…” and setup comments in checkout runner tests. |
| internal/cli/branch_test.go | Removed redundant test doc comments and subcommand verification headings. |
| AGENTS.md | Added Landmine #10 documenting minimal, non-decorative comment expectations. |
Comments suppressed due to low confidence (2)
internal/git/git_more_test.go:669
- A short rationale comment here would help preserve the non-obvious constraint this test covers: untracked directories from
git status --porcelain(e.g.,?? newdir/) require thegit cleancode path to include directory removal (the-dbehavior). Without that context, it’s easy to regress the implementation while still thinking “untracked files” are covered.
internal/cli/upgrade_test.go:706 - The removed header comment here explained why these tests call
httpUpgradeClient.downloadBytesvia a realhttptest.Server(to exercise the realio.LimitReader/ max-size enforcement path rather than the fake client). That rationale is non-obvious and worth keeping as a short comment so future changes don’t accidentally weaken the coverage.
func TestHTTPDownloadBytesOversizedRejected(t *testing.T) {
oversized := bytes.Repeat([]byte("x"), int(maxBundleSize)+1)
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
if _, err := w.Write(oversized); err != nil {
t.Errorf("write oversized payload: %v", err)
}
}))
defer srv.Close()
uc := &httpUpgradeClient{client: newTestHTTPClient(srv)}
_, err := uc.downloadBytes(srv.URL + "/bundle")
Comment on lines
345
to
350
| func TestForcePush(t *testing.T) { | ||
| // Create a bare repo to act as origin. | ||
| bareDir := t.TempDir() | ||
| mustRunGit(t, bareDir, "init", "--bare", "--initial-branch=main") | ||
|
|
||
| // Create a working repo and push an initial commit. | ||
| workDir := initRepo(t) | ||
| mustRunGit(t, workDir, "config", "user.email", "test@example.com") |
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
====,----,───)// Phase X:)AGENTS.mdrule feat: add --repo/-r flag to gitm commit #10 on minimal comment styleScope
32 files across e2e tests, unit tests, internal packages, and CLI code cleaned.
Changes
AGENTS.mddocumenting new ruleValidation
go test ./... -v -race -timeout 60s✅ (406 passing)make lint✅gofmtappliedRationale
Decorative comments (banners, phase labels, line repetition) add visual clutter without explaining why code exists. Per new
AGENTS.mdrule #10, we keep only comments that add context—safety rationale, algorithm explanation, TTY limitations, etc. This improves code readability and makes meaningful comments stand out.