47 lines
3.3 KiB
Markdown
47 lines
3.3 KiB
Markdown
---
|
|
status: resolved
|
|
parent: "[[040-implement-skill-matt-skills-discovery-research]]"
|
|
claimed-by: "019fbb66-80d0-7a01-8ac9-01255adfc7b2"
|
|
claimed-at: "2026-07-31T23:47:59-04:00"
|
|
blocked-by:
|
|
- "[[040-implement-skill-matt-skills-discovery-research]]"
|
|
tags:
|
|
- ticket/research
|
|
---
|
|
|
|
# Implement skill Matt code review research
|
|
|
|
## Question
|
|
|
|
What should the migrated local `implement` skill learn from Matt Pocock's `skills/engineering/code-review` workflow?
|
|
|
|
## Findings
|
|
|
|
Matt's `code-review` reviews the diff between `HEAD` and a fixed point supplied by the user, using three-dot diff against the merge base and listing commits with `git log <fixed-point>..HEAD --oneline`.
|
|
This branch-level review fits a committed implementation branch better than the old local `/review-uncommitted`, which reviews `git diff HEAD` before commit.
|
|
Source: `https://raw.githubusercontent.com/mattpocock/skills/main/skills/engineering/code-review/SKILL.md`.
|
|
|
|
Matt's review has two axes: Standards and Spec.
|
|
It deliberately keeps the axes separate so spec fidelity cannot hide code-quality problems and standards compliance cannot hide incorrect behavior.
|
|
The old local review adds a Risk axis.
|
|
A migrated `implement` skill should decide whether to run uncommitted three-axis review before commit, branch-level two-axis review after commit, or both.
|
|
Sources: `https://raw.githubusercontent.com/mattpocock/skills/main/skills/engineering/code-review/SKILL.md`, `/home/alexion/wrk/dotfiles/modules/agents/claude-code/skills/review-uncommitted/SKILL.md`.
|
|
|
|
Matt's review locates spec sources from issue references in commits, explicit arguments, matching PRD/spec files, or user clarification, and it asks for tracker setup if issue tracker docs are missing.
|
|
For Wayfinder, the explicit task ticket artifact should be the primary spec source, with parent map or artifacts loaded only when needed.
|
|
Source: `https://raw.githubusercontent.com/mattpocock/skills/main/skills/engineering/code-review/SKILL.md`.
|
|
|
|
Matt's Standards axis includes a Fowler-smell baseline and documented-standard override, matching the old local review's standards approach.
|
|
This should be preserved because it gives review teeth even when the repository has sparse standards docs.
|
|
Sources: `https://raw.githubusercontent.com/mattpocock/skills/main/skills/engineering/code-review/SKILL.md`, `/home/alexion/wrk/dotfiles/modules/agents/claude-code/skills/review-uncommitted/SKILL.md`.
|
|
|
|
The packaged local skills repository does not currently contain a direct `code-review` or `review-uncommitted` counterpart.
|
|
Its packaged `axi-review` skill is a black-box CLI conformance review against AXI principles, not a code diff review, so it is not a substitute for Matt's `code-review` in the migrated `implement` workflow.
|
|
A migrated packaged `implement` must either package or depend on a new local review skill, inline a review gate, or call an external review workflow explicitly.
|
|
Source: `/home/alexion/wrk/skills/skills/axi-review/SKILL.md`.
|
|
|
|
## Conclusion
|
|
|
|
Matt's `code-review` suggests branch-level review against a fixed point may be a better final gate than uncommitted review alone.
|
|
The migrated `implement` skill should preserve separate Standards and Spec axes, keep the smell baseline, use the Wayfinder task as spec source, and decide whether to retain the local Risk axis and pre-commit timing.
|