Skip to content

docs: clarify comment style and commit bisectability guidance - #7490

Merged
PastaPastaPasta merged 1 commit into
dashpay:developfrom
PastaPastaPasta:claude/comment-commit-guidelines-b0cf55
Jul 28, 2026
Merged

docs: clarify comment style and commit bisectability guidance#7490
PastaPastaPasta merged 1 commit into
dashpay:developfrom
PastaPastaPasta:claude/comment-commit-guidelines-b0cf55

Conversation

@PastaPastaPasta

@PastaPastaPasta PastaPastaPasta commented Jul 27, 2026

Copy link
Copy Markdown
Member

Issue being fixed or feature being added

Agent guidance in CLAUDE.md/AGENTS.md didn't address comment verbosity, allow non-building commits (e.g. a regression test before its fix), or point at the CI check that validates PR titles.

What was done

  • Added guidance that code should be readable on its own; comments shouldn't restate the code and are reserved for things that genuinely need explaining.
  • Relaxed the atomic-commit rule: commits should generally build and pass tests, but an intentionally non-building commit is fine if called out explicitly.
  • Pointed the PR-title rule at .github/workflows/semantic-pull-request.yml, which enforces the valid Conventional Commit types/scopes.
  • Kept CLAUDE.md and AGENTS.md in sync as required.

How Has This Been Tested

Docs-only change; diffed the two files to confirm they still match.

Breaking Changes

None.

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone

Discourage comments that merely restate code, favoring readable code itself. Relax atomic-commit rule to allow an intentionally non-building commit (e.g. a regression test preceding its fix) when called out explicitly. Also point the PR-title rule at the CI linter (semantic-pull-request.yml) that enforces valid types/scopes.
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Updated AGENTS.md and CLAUDE.md with guidance to prefer self-explanatory code, reserve comments for non-obvious details, and document intentional build or test breaks in atomic commits.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

  • dashpay/dash#7410: Updates the same agent documentation sections with overlapping comment and atomic-commit guidance.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the docs-only guidance updates about comments and atomic-commit bisectability.
Description check ✅ Passed The description accurately describes the synchronized guidance changes made in CLAUDE.md and AGENTS.md.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thepastaclaw

thepastaclaw commented Jul 27, 2026

Copy link
Copy Markdown

✅ Final review complete — no blockers (commit d3ed9c7)

@PastaPastaPasta
PastaPastaPasta force-pushed the claude/comment-commit-guidelines-b0cf55 branch from fd065c9 to d3ed9c7 Compare July 27, 2026 17:00

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Final validation — Codex + Sonnet

This docs-only change keeps AGENTS.md and CLAUDE.md byte-identical, and the referenced semantic-pull-request workflow does enforce the documented Conventional Commit types and scopes. The proposed distinction between test-failing and non-building commits would reverse the explicitly stated policy relaxation rather than correct a factual error, so there are no actionable findings.

Source: reviewers gpt-5.6-sol (general, dash-core-commit-history) and claude-sonnet-5 (general, dash-core-commit-history); final verifier gpt-5.6-sol via the policy-permitted Claude Code fallback after the primary claude-sonnet-5 verifier returned fenced, unparseable JSON.

Review provenance

  • Codex reviewers: gpt-5.6-sol — general (completed), gpt-5.6-sol — dash-core-commit-history (completed)
  • Verifier: gpt-5.6-sol — final-verifier (fallback)
  • Sonnet reviewers: claude-sonnet-5 — general (completed), claude-sonnet-5 — dash-core-commit-history (completed)

@PastaPastaPasta
PastaPastaPasta merged commit 52edc6e into dashpay:develop Jul 28, 2026
42 of 45 checks passed
@UdjinM6 UdjinM6 added this to the 24 milestone Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants