Skip to content

chore: condense Claude comments - #1158

Merged
mbevc1 merged 5 commits into
mainfrom
20260910_claude_comments
Sep 10, 2026
Merged

mbevc1 merged 5 commits into
mainfrom
20260910_claude_comments

Conversation

@mbevc1

@mbevc1 mbevc1 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Add comment style skill and try to prevent proliferating code comments.

Checklist

@github-actions github-actions Bot added the chore label Sep 10, 2026
@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mbevc1's task in 2m 23s —— View job


Review — PR #1158

  • Gather context (diff, CLAUDE.md, sibling skill)
  • Static analysis (go build, go vet, go mod tidy)
  • Review changes
  • Post review

Static analysis

Check Result
go build ./... ✅ clean
go vet ./... ✅ clean
go mod tidy + git diff go.mod go.sum ✅ no drift

Docs-only change (.claude/skills/comment-style/SKILL.md, +59), so no Go surface is touched — the checks are a formality here. File ends with a trailing newline; frontmatter is valid YAML.

Resolved since the last run

  • ✅ Duplicate ## Examples heading fixed in 46b06a5a.
  • ✅ Godoc carve-out (lines 17-18) reconciles the skill with CLAUDE.md:119-121.
  • ✅ Rule 2 (lines 29-32) no longer contradicts rule 4 and the Good example.

Still open

1. Example label no longer matches the rule it illustratesSKILL.md:49

Rule 2 now explicitly allows a counterfactual when it is the reason, and the Good version on lines 54-56 keeps the same "without this" framing. So "history and justification" mislabels what's actually wrong: three lines of narration around one load-bearing fact — i.e. the Precise means rule.

Bad — one fact buried in three lines of narration:

Fix this →

2. #5765 renders as a live link to the wrong thingSKILL.md:42, SKILL.md:56

GitHub autolinks bare #N, so in this repo #5765 points at kosli-dev/cli#5765, which doesn't exist (the repo is around #1158). An agent copying the pattern learns to invent refs. (#NNNN) as a placeholder avoids that.

3. Mixed comment markers (nit) — four examples use #, line 59 uses //

Deliberate now that line 59 demonstrates the Go godoc exception, but a half-sentence saying markers are illustrative would remove the ambiguity. Non-blocking.

4. Discoverability (nit) — the skill isn't referenced from CLAUDE.md

CLAUDE.md:49 points at new-command explicitly; CLAUDE.md:119-121 restates comment guidance inline without mentioning this skill. Given the PR's stated goal — prevent proliferating code commentsCLAUDE.md is the higher-leverage hook, since it loads on every session while a skill only fires on description match. Having the rule stated in two places also invites drift. Delegating lines 119-121 to .claude/skills/comment-style/ would fix both. Worth considering, not blocking.

What's good

The rules are ordered and each is falsifiable, which is what makes this usable by an agent rather than aspirational. The paired Bad/Good examples showing the same fact at two lengths teach compression rather than just prohibition — more useful than a list of bans. The description frontmatter enumerates concrete trigger phrasings, which is what actually drives skill activation. The godoc exception and the rule-2 counterfactual carve-out both came from real contradictions and were resolved without weakening the default.

Verdict: approve-with-nits. Only item 1 is a real correctness problem in the doc; the rest are polish.
· branch 20260910_claude_comments

@mbevc1
mbevc1 enabled auto-merge (squash) September 10, 2026 07:11
Comment thread .claude/skills/comment-style/SKILL.md
Comment thread .claude/skills/comment-style/SKILL.md
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Comment thread .claude/skills/comment-style/SKILL.md Outdated
Comment thread .claude/skills/comment-style/SKILL.md Outdated
mbevc1 and others added 2 commits September 10, 2026 08:39
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Comment thread .claude/skills/comment-style/SKILL.md Outdated
Comment on lines +50 to +53
Bad — history and justification burying one fact:
`# Without this the analytics preflight only ran from the client-triggered background`
`# check, so an operator who navigated away before the initiate response landed got a`
`# plan with no analytics check recorded...`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The label no longer matches the rule it illustrates. Rule 2 now explicitly allows a counterfactual when it is the reason, so "history and justification" mislabels what's wrong here — the Good version two lines down keeps the same counterfactual. What this example actually demonstrates is the Precise means rule: three lines of narration around one load-bearing fact.

Suggested change
Bad — history and justification burying one fact:
`# Without this the analytics preflight only ran from the client-triggered background`
`# check, so an operator who navigated away before the initiate response landed got a`
`# plan with no analytics check recorded...`
Bad — one fact buried in three lines of narration:
`# Without this the analytics preflight only ran from the client-triggered background`
`# check, so an operator who navigated away before the initiate response landed got a`
`# plan with no analytics check recorded...`

Comment on lines +55 to +57
Good — same fact, stated precisely (ref kept as a pointer):
`# Record the analytics preflight here too — the post-initiate auto-run misses it`
`# when the client navigates away before the initiate response lands (#5765).`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two low-priority points about the examples as a set:

  1. #5765 renders as a live link to the wrong thing. GitHub autolinks bare #N refs, so in this repo #5765 (also on line 42) points at kosli-dev/cli#5765, which doesn't exist — the repo is around chore: condense Claude comments #1158. An agent copying the pattern would learn to invent refs. A placeholder avoids that: (#NNNN) or (kosli-dev/cli#1158).
  2. Mixed comment markers. Four examples use #, the last uses //. The skill is language-agnostic by design, so this may be deliberate — but every comment it will actually be applied to in this repo is //, and the godoc carve-out on lines 17-18 is Go-specific. Either convert them all to //, or note explicitly that markers are illustrative.

Neither blocks merge.

Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
@mbevc1
mbevc1 merged commit efd606b into main Sep 10, 2026
22 checks passed
@mbevc1
mbevc1 deleted the 20260910_claude_comments branch September 10, 2026 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants