Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .agent_instructions/code_style.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@
- As we support multiple schedulers, these should use their own assembly.
- As we support multiple locking providers, these should use their own assembly.
- Default to a class per source file approach, unless one class clearly exists as the details of another.
- Comments
- Do not put links or references to issues or PRs (e.g. `(issue #4089)`, `#4207`) in code or XML doc comments. The traceability lives in git history and the ADR, not in the source.
- A comment is for code that needs explanation — typically the *why* behind a non-obvious decision. Do not narrate *what* the code does, restate the code, or describe the change relative to how the code used to be.
- Inline comments are not notes to a reviewer. Context about the fix, the alternatives considered, the previous behaviour, or the history of a change is reviewer-facing rationale and belongs in the ADR, not in an inline comment. A future maintainer reading the code only needs what helps them understand the code as it now stands.
- Keep comments concise. Prefer one tight line over a paragraph.
- Comments — the rules are in [documentation.md § Comments in Code](documentation.md#comments-in-code). In short:
- `///` XML documentation on public and protected members of public types only.
- On anything else, at most a `//` or `/* */` comment before the signature, when the method is complex or guards against a common trap.
- No comments inside method bodies, as a rule: extract a method to express intent instead (except `//Arrange` `//Act` `//Assert` in tests).
- No issue, PR or spec references, no change history, no notes to a reviewer. Those belong in the ADR and git history.

34 changes: 28 additions & 6 deletions .agent_instructions/documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,9 @@
public string Status { get; set; }
```

- Use `<remarks>` for advice to developers or LLMs working with the code directly. Include information on how the method is implemented where it is not obvious from the code or significant design decisions have been made. Consider what you would want to know if maintaining this method. Use `<see href="">` if you need to link to external documentation. This can also be used for more detailed information than could be included in the `<summary>`.
- Prefer to use good variable and method names to express intent, over inline comments.
- Use the refactoring "Extract Method To Express Intent" to encapsulate code in a named method that explains intent, over using a comment.
- Do not add comments for what may be easily inferred from the code.
- In tests you may use //Arrange, //Act, //Assert.
- If code has a complex algorithm or non-obvious implementation, prefer to use `/// <remarks>`
- The elements divide into two kinds:
- **How to use the API** — `<summary>`, `<param>`, `<typeparam>`, `<returns>`, `<value>`, `<exception>`. What it is for and what to pass to get the behaviour you want.
- **Notes to the user** — `<remarks>`. Caveats, side effects, threading or ordering constraints, and how it is implemented where that affects how you use it, or where a significant design decision has been made. Use `<see href="">` if you need to link to external documentation. This can also be used for more detailed information than could be included in the `<summary>`.
- Example:

```csharp
Expand All @@ -46,6 +43,31 @@ public string Status { get; set; }
- Documentation comments should be changed when APIs change.
- Document new features and changes in the Docs repository of the BrighterCommand organization.

## Comments in Code

Where a comment may go depends on what it is attached to.

| Where | What is allowed |
|-------|-----------------|
| Public and protected members of public types | `///` XML documentation, as above. Required. |
| Internal or private types and members | No XML documentation. Where a method is complex to understand, or protects against a common trap, a `//` or `/* */` comment **immediately before the signature** saying what it does or which trap it guards against. Otherwise nothing. |
| Inside a method body | **No comments, as a rule.** |

- Express intent with code, not comments, inside a method body. If a block needs a comment to be understood, use the refactoring "Extract Method To Express Intent": give the block a name that says what it does, and, if the name is not enough, put the comment above the new method's signature.
- The same goes for the smaller refactorings: Extract Variable for a condition that needs explaining, a named constant for a magic literal, a rename for a variable whose name needs a comment.
- A comment that warns about a trap ("must be called before…", "can be null when…", "do not reorder…") often hides a refactoring that removes the trap: make the ordering impossible to get wrong, make the type say it can be null, or guard it with an assertion.
- The exceptions inside a method body are:
- `//Arrange`, `//Act`, `//Assert` in tests.
- The reason on an analyser suppression (`#pragma warning disable`, `// ReSharper disable`), on the same or preceding line.
- A single line marking a trap that cannot be extracted, such as a workaround for a bug in a third-party library at one call site. This is rare; reach for extraction first.
- Every comment, wherever it sits:
- Explains the code as it now stands. It does not narrate *what* the code does, restate the code, or describe the change relative to how the code used to be.
- Is not a note to a reviewer. The fix, the alternatives considered, the previous behaviour and the history of a change are reviewer-facing rationale; they belong in the ADR or the commit message.
- Carries no links or references to issues, PRs or specs (e.g. `(issue #4089)`, `#4207`, `Spec 0027`). The traceability lives in git history and the ADR. Referring to an ADR by number where the design decision explains the code is fine.
- Is concise. Prefer one tight line over a paragraph.
- Delete commented-out code. Git remembers it.
- `/refactor:tidy-comments` brings existing code into line with these rules.

## Architecture Decision Records

**Recommended Tool**: Use the `/adr <title>` command (see [.claude/commands/adr/adr.md](../../.claude/commands/adr/adr.md)) to create properly formatted ADRs. This automates numbering, template application, and spec linking.
Expand Down
6 changes: 6 additions & 0 deletions .agent_instructions/skills_overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Skills are slash commands that automate multi-step workflows and enforce Brighte
| `/test-first` | TDD with an approval gate, armed by default | `/test-first <behavior description>` |
| `/tidy-first` | Separate refactoring from features | `/tidy-first <change description>` |
| `/refactor:smells` | Find code smells in completed work, schedule refactorings | `/refactor:smells [base-ref \| base..head \| path...]` |
| `/refactor:tidy-comments` | Bring comments into line with the comment rules; surface refactorings a comment side-stepped | `/refactor:tidy-comments [base-ref \| base..head \| path...]` |
| `/adr` | Create Architecture Decision Record | `/adr <title>` |
| `/bugfix:*` | Diagnosis-first bug workflow (Confirm gate) | `/bugfix:triage [issue \| description]` |

Expand Down Expand Up @@ -75,6 +76,11 @@ Skills are slash commands that automate multi-step workflows and enforce Brighte
• Opt-in, after a unit of work is green: find Fowler's code smells
• Reports and schedules refactorings; does not change code

/refactor:tidy-comments [base-ref | base..head | path...]
• Delete, trim or move comments that break documentation.md § Comments in Code
• Replaces explanatory comments with Extract Method / Variable / Rename
• Reports comments that side-stepped a real refactoring, and schedules it

📋 ARCHITECTURE DECISIONS
/adr <title>
• Auto-numbered, properly formatted ADRs
Expand Down
3 changes: 3 additions & 0 deletions .claude/commands/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Skills are invoked using slash commands in Claude Code:
/adr <title> # Create Architecture Decision Record
/tidy-first <change description> # Separate structural from behavioral changes
/refactor:smells [base-ref | base..head | path...] # Find code smells in completed work, schedule refactorings
/refactor:tidy-comments [base-ref | base..head | path...] # Replace comments with intent; find side-stepped refactorings
```

## Available Skills
Expand Down Expand Up @@ -151,6 +152,7 @@ Skills are invoked using slash commands in Claude Code:
- **`/test-first`** - TDD with an approval gate, armed by default (shift it with `/spec:gear`)
- **`/tidy-first`** - Safe refactoring workflow
- **`/refactor:smells`** - Opt-in code smell review of completed work; schedules refactorings
- **`/refactor:tidy-comments`** - Brings comments into line with the comment rules; schedules refactorings a comment side-stepped
- **`/bugfix:*`** - Diagnosis-first bug workflow (Triage → Confirm → Test-first → Fix → Verify)

### Documentation Skills
Expand Down Expand Up @@ -241,6 +243,7 @@ These skills enforce practices documented in `.agent_instructions/`:
| `/adr` | ADR creation standards | [documentation.md](../../.agent_instructions/documentation.md) lines 49-62 |
| `/tidy-first` | Structural/behavioral separation | [code_style.md](../../.agent_instructions/code_style.md) lines 74-83 |
| `/refactor:smells` | Fowler's code smells, calibrated to Brighter's design principles | [design_principles.md](../../.agent_instructions/design_principles.md) |
| `/refactor:tidy-comments` | Where comments may go, and Extract Method over comments | [documentation.md](../../.agent_instructions/documentation.md) → "Comments in Code" |

The first three make **mandatory workflows enforceable** rather than just documented. `/refactor:smells` is opt-in.

Expand Down
5 changes: 5 additions & 0 deletions .claude/commands/bugfix/fix.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ Read the suspect code (Confirmed Root Cause + Suspected Location).
"improvements", no default changes (CLAUDE.md → "Change Scope").
2. Follow `.agent_instructions/code_style.md` (naming, RDD, nullable, XML docs on public members,
MIT header on new files) and `.agent_instructions/documentation.md`.
- **Do not explain the fix in a comment.** The root cause, the previous behaviour and why this
fix was chosen belong in `bugfix.md`, the commit message and the PR — not in the code. No
comments inside method bodies, and no issue or bugfix numbers anywhere in the source
(`documentation.md` § *Comments in Code*). If the fixed code is hard to read without a
comment, extract a method whose name says what it guards against.
3. If the Scope Notes listed additional defects, fix those too — they share the confirmed cause
and each has its own red test from `/bugfix:test`.
4. **Cross-backend parity**: if the cause touches one backend (MSSQL/PostgreSQL/MySQL/SQLite/
Expand Down
23 changes: 23 additions & 0 deletions .claude/commands/refactor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,29 @@ marked `structural` (a `/tidy-first` job) or `public API change` (needs a decisi
/refactor:smells src/Paramore.Brighter/CommandProcessor.cs
```

### `/refactor:tidy-comments [base-ref | base..head | path...]`

Brings the comments in a unit of work into line with
[documentation.md § Comments in Code](../../../.agent_instructions/documentation.md#comments-in-code):
XML docs on public members only, at most a comment above the signature elsewhere, and none inside
method bodies.

**Purpose**: Remove comments that narrate the code, record its history, cite issues or specs, or talk
to a reviewer; replace comments that explain a block or expression with Extract Method, Extract
Variable, a named constant or a rename. Comments that were written *instead of* a refactoring
("must be called first", "can be null when…") are reported and scheduled, as `/refactor:smells` does.

**It changes code, structurally only**: a comment-only `refactor:` commit, then an extraction
`refactor:` commit, with tests green and no public signature changed. Reviewer rationale missing from
the ADR is shown to you rather than deleted.

**Usage:**
```bash
/refactor:tidy-comments # comments this branch added or changed
/refactor:tidy-comments HEAD~3 # comments changed since a ref
/refactor:tidy-comments src/Paramore.Brighter/CommandProcessor.cs # every comment in a file with no changes
```

## The Core Principle

From [.agent_instructions/code_style.md](../../../.agent_instructions/code_style.md):
Expand Down
4 changes: 3 additions & 1 deletion .claude/commands/refactor/smells.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@ cases. **Do not report** these as smells:
- **Primitives used for serialisation or interoperability**, which the design principles allow.
- **XML documentation comments** on public members — required by
[documentation.md](../../../.agent_instructions/documentation.md). The Comments smell is about
comments inside method bodies that narrate the code.
comments inside method bodies that narrate the code. Where the change under review has many of
them, report one finding and point to `/refactor:tidy-comments`, which applies the full rules,
rather than listing every comment.
- **Sync/async pairs** (`Send`/`SendAsync`, `IAmA*`/`IAmA*Async`) with parallel bodies — the duplication
is deliberate across the sync and async pipelines. Report it only where shared logic could be
extracted without crossing that line.
Expand Down
Loading
Loading