Skip to content

Commit 14c13c5

Browse files
mattpocockclaude
andcommitted
Rename review skill to code-review and promote to engineering
Rename the in-progress `review` skill to `code-review` and move it from `skills/in-progress/` into the promoted `skills/engineering/` bucket. - Add it to `.claude-plugin/plugin.json` and the Model-invoked sections of the top-level and Engineering READMEs; drop it from the in-progress README. - Add a docs page at `docs/engineering/code-review.md`. - Point `/implement` and its docs at `/code-review`. - Update the smell-baseline changeset reference; add a changeset for the move. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5a41915 commit 14c13c5

10 files changed

Lines changed: 59 additions & 5 deletions

File tree

.changeset/code-review-promote.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"mattpocock-skills": minor
3+
---
4+
5+
Rename the in-progress **`review`** skill to **`code-review`** and promote it from `in-progress/` to the `engineering/` bucket. It now ships in the plugin, is listed in the top-level and Engineering READMEs (Model-invoked), and has a human-facing docs page at `docs/engineering/code-review.md`. The `/implement` skill and docs now point at `/code-review`.

.changeset/review-smell-baseline.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
"mattpocock-skills": patch
33
---
44

5-
Give the in-progress **`review`** skill an always-on Fowler smell baseline on its Standards axis. A curated ~12 high-signal "Bad Smells in Code" (Mysterious Name, Duplicated Code, Feature Envy, Data Clumps, Primitive Obsession, Repeated Switches, Shotgun Surgery, Divergent Change, Speculative Generality, Message Chains, Middle Man, Refused Bequest) are inlined into `SKILL.md` as a fixed baseline alongside whatever the repo documents — not a new third axis. Two binding rules keep it safe: a documented repo standard overrides the baseline, and every smell is reported as a judgement call, never a hard violation.
5+
Give the in-progress **`code-review`** skill an always-on Fowler smell baseline on its Standards axis. A curated ~12 high-signal "Bad Smells in Code" (Mysterious Name, Duplicated Code, Feature Envy, Data Clumps, Primitive Obsession, Repeated Switches, Shotgun Surgery, Divergent Change, Speculative Generality, Message Chains, Middle Man, Refused Bequest) are inlined into `SKILL.md` as a fixed baseline alongside whatever the repo documents — not a new third axis. Two binding rules keep it safe: a documented repo standard overrides the baseline, and every smell is reported as a judgement call, never a hard violation.

.claude-plugin/plugin.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
"./skills/engineering/prototype",
1414
"./skills/engineering/domain-modeling",
1515
"./skills/engineering/codebase-design",
16+
"./skills/engineering/code-review",
1617
"./skills/productivity/grill-me",
1718
"./skills/productivity/grilling",
1819
"./skills/productivity/handoff",

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ Skills I use daily for code work.
164164
- **[tdd](./skills/engineering/tdd/SKILL.md)** — Test-driven development with a red-green-refactor loop. Builds features or fixes bugs one vertical slice at a time.
165165
- **[domain-modeling](./skills/engineering/domain-modeling/SKILL.md)** — Actively build and sharpen a project's domain model — challenge terms against the glossary, stress-test with edge-case scenarios, and update `CONTEXT.md` and ADRs inline.
166166
- **[codebase-design](./skills/engineering/codebase-design/SKILL.md)** — Shared discipline and vocabulary for designing deep modules: a lot of behaviour behind a small interface, placed at a clean seam, testable through that interface.
167+
- **[code-review](./skills/engineering/code-review/SKILL.md)** — Two-axis review of the diff since a fixed point: **Standards** (does it follow the repo's coding standards, plus a Fowler smell baseline?) and **Spec** (does it faithfully implement the originating issue/PRD?), run as parallel sub-agents so neither pollutes the other.
167168

168169
### Productivity
169170

docs/engineering/code-review.md

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
Quickstart:
2+
3+
```bash
4+
npx skills add mattpocock/skills --skill=code-review
5+
```
6+
7+
```bash
8+
npx skills update code-review
9+
```
10+
11+
[Source](https://github.com/mattpocock/skills/tree/main/skills/engineering/code-review)
12+
13+
## What it does
14+
15+
`code-review` reviews the diff between `HEAD` and a fixed point you supply — a commit, branch, tag, or merge-base — along two separate axes: **Standards** (does the code follow this repo's documented conventions?) and **Spec** (does it implement what the originating issue or PRD asked for?). It runs each axis as its own parallel sub-agent and reports them side by side. It never merges or re-ranks the two sets of findings — keeping them separate is the whole point, because a change can pass one axis and fail the other, and a single blended verdict lets one mask the other.
16+
17+
## When to reach for it
18+
19+
Type `/code-review`, or the agent reaches for it automatically when you ask to review a branch, a PR, work-in-progress changes, or anything "since X".
20+
21+
Reach for this when there is a diff to judge against a known-good point and you want the two questions — *is it built right?* and *is it the right thing?* — answered independently. It runs at the end of the build loop; for actually writing the code test-first, use [tdd](https://aihero.dev/skills-tdd), and for building a whole spec into code use [implement](https://aihero.dev/skills-implement), which runs its own `/code-review` pass before committing.
22+
23+
## Prerequisites
24+
25+
The **Spec** axis needs somewhere to find the originating spec — an issue reference in the commit messages, a path you pass in, or a PRD under `docs/`/`specs/`. That issue-tracker wiring comes from [setup-matt-pocock-skills](https://aihero.dev/skills-setup-matt-pocock-skills); without a spec the Spec axis simply skips and says so. The **Standards** axis needs nothing set up — it always carries a built-in Fowler smell baseline even in a repo that documents no conventions.
26+
27+
## Two axes, never merged
28+
29+
The defining idea is the **two axes**. **Standards** asks whether the diff conforms to how this repo writes code — its `CODING_STANDARDS.md` or `CONTRIBUTING.md`, plus a fixed baseline of ~12 Fowler code smells (Mysterious Name, Duplicated Code, Feature Envy, Data Clumps, …). Two rules keep the baseline safe: a documented repo standard always overrides it, and every smell is a judgement call, never a hard violation. **Spec** asks the orthogonal question — does the code do what the issue or PRD actually asked, without missing requirements or smuggling in scope creep?
30+
31+
They run as parallel sub-agents so neither pollutes the other's context, and the final report presents them under separate `## Standards` and `## Spec` headings with a per-axis summary. There is deliberately no single winner across axes.
32+
33+
## It's working if
34+
35+
- It pins and confirms the fixed point first (`git rev-parse`), failing fast on a bad ref or empty diff rather than inside the sub-agents.
36+
- Standards and Spec findings arrive in two distinct blocks, each citing its source — a repo standard or baseline smell for one, a quoted spec line for the other.
37+
- When no spec can be found, the Spec axis reports "no spec available" instead of inventing requirements.
38+
39+
## Where it fits
40+
41+
`code-review` is the review step at the tail of the main build chain:
42+
43+
```txt
44+
grill-with-docs → to-prd → to-issues → implement → code-review
45+
```
46+
47+
Its closest neighbour is [implement](https://aihero.dev/skills-implement), which drives the build and calls this as its own review pass before committing; upstream, the spec it checks against is produced by [to-prd](https://aihero.dev/skills-to-prd) and [to-issues](https://aihero.dev/skills-to-issues). When you're unsure which skill or flow fits, [ask-matt](https://aihero.dev/skills-ask-matt) routes you.

docs/engineering/implement.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,4 @@ Around that core it keeps the loop tight — typecheck often, run single test fi
3636
grill-with-docs → to-prd → to-issues → implement
3737
```
3838

39-
Reach for it after the work has been specced and sequenced, not before. Its key neighbours are [to-issues](https://aihero.dev/skills-to-issues), which produces the independently-grabbable tickets it works through, and [tdd](https://aihero.dev/skills-tdd), which it drives internally to write the tests at each seam before running its own `/review` pass and committing. When you're unsure which skill or flow fits, [ask-matt](https://aihero.dev/skills-ask-matt) routes you.
39+
Reach for it after the work has been specced and sequenced, not before. Its key neighbours are [to-issues](https://aihero.dev/skills-to-issues), which produces the independently-grabbable tickets it works through, and [tdd](https://aihero.dev/skills-tdd), which it drives internally to write the tests at each seam before running its own `/code-review` pass and committing. When you're unsure which skill or flow fits, [ask-matt](https://aihero.dev/skills-ask-matt) routes you.

skills/engineering/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,4 @@ Model- or user-reachable (rich trigger phrasing so the model can reach for them)
2424
- **[tdd](./tdd/SKILL.md)** — Test-driven development with a red-green-refactor loop. Builds features or fixes bugs one vertical slice at a time.
2525
- **[domain-modeling](./domain-modeling/SKILL.md)** — Actively build and sharpen a project's domain model — challenge terms, stress-test with scenarios, update `CONTEXT.md` and ADRs inline.
2626
- **[codebase-design](./codebase-design/SKILL.md)** — Shared discipline and vocabulary for designing deep modules: small interfaces, clean seams, testable through the interface.
27+
- **[code-review](./code-review/SKILL.md)** — Two-axis review of the diff since a fixed point: **Standards** (does it follow the repo's coding standards, plus a Fowler smell baseline?) and **Spec** (does it faithfully implement the originating issue/PRD?), run as parallel sub-agents.
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
---
2-
name: review
2+
name: code-review
33
description: Review the changes since a fixed point (commit, branch, tag, or merge-base) along two axes — Standards (does the code follow this repo's documented coding standards?) and Spec (does the code match what the originating issue/PRD asked for?). Runs both reviews in parallel sub-agents and reports them side by side. Use when the user wants to review a branch, a PR, work-in-progress changes, or asks to "review since X".
44
---
55

skills/engineering/implement/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,6 @@ Use /tdd where possible, at pre-agreed seams.
1010

1111
Run typechecking regularly, single test files regularly, and the full test suite once at the end.
1212

13-
Once done, use /review to review the work.
13+
Once done, use /code-review to review the work.
1414

1515
Commit your work to the current branch.

skills/in-progress/README.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ Skills that are still being developed. They're not ready to ship — expect roug
44

55
- **[decision-mapping](./decision-mapping/SKILL.md)** — Turn a loose idea into a sequenced map of investigation tickets, then drive them to resolution one at a time. User-invoked.
66
- **[loop-me](./loop-me/SKILL.md)** — Grill yourself into implementable workflow specs over multiple sessions, using the current directory as a stateful workspace. User-invoked.
7-
- **[review](./review/SKILL.md)** — Review changes since a fixed point along two parallel axes: **Standards** (does the diff follow the repo's coding standards?) and **Spec** (does the diff faithfully implement the originating issue/PRD?).
87
- **[wizard](./wizard/SKILL.md)** — Generate an interactive bash wizard that walks a human through a manual procedure (setup, a one-off migration, a state transition) — opening URLs, capturing values, writing `.env` and GitHub Actions secrets. User-invoked.
98
- **[writing-beats](./writing-beats/SKILL.md)** — Shape an article as a journey of beats, choose-your-own-adventure style. Pick a starting beat, write only that beat, then pivot to the next, until the article reaches a natural end.
109
- **[writing-fragments](./writing-fragments/SKILL.md)** — Grilling session that mines you for fragments — heterogeneous nuggets of writing — and appends them to a single document as raw material for a future article.

0 commit comments

Comments
 (0)