diff --git a/skills/code-review/SKILL.md b/skills/code-review/SKILL.md index b416ed8..d344759 100644 --- a/skills/code-review/SKILL.md +++ b/skills/code-review/SKILL.md @@ -10,7 +10,7 @@ Review changed code against `references/smells.md` (from the book *Refactoring*) ## When to run 1. **When a file change is complete**: one file or one related group of files is done. -2. **When an implementation is complete**: all todos of a work package are done (called by `jsc-sdlc:implement` step 6). +2. **When an implementation is complete**: all todos of a work package are done. This is the call site in `jsc-sdlc:implement` — the end of a work package, after its last todo and before it is committed and turned into a PR. ## Division of labor @@ -19,7 +19,7 @@ Review changed code against `references/smells.md` (from the book *Refactoring*) ## Steps -1. Get the review scope: `git diff` (uncommitted changes) or `git diff {base}...HEAD` (against the base branch when an implementation is complete); list the changed files. +1. Get the review scope: `git diff` (uncommitted changes) or `git diff {base}...HEAD` (against the base branch when an implementation is complete); list the changed files. An empty diff → report the literal 「無發現」 and stop here; spawn no sub agent. Completion condition: the changed-file list is non-empty and reported, or the run already ended with 「無發現」. 2. Review in six groups, and every group **MUST run as a sub agent**; the six groups may run in parallel: | Group | Scope | @@ -32,8 +32,10 @@ Review changed code against `references/smells.md` (from the book *Refactoring*) | 6 Shallow Module | smells.md group 6 | Instructions for each sub agent: read only, change nothing; check every changed line and its enclosing function or class against the group's definitions and detection signals in smells.md; report each finding as `file:line`, smell name, severity (高、中、低 per the smells.md scale), one sentence of evidence, and the suggested refactoring. Findings are reported in Traditional Chinese. -3. Merge the six groups' findings: deduplicate (when one location hits several groups, merge and list every smell), then sort by severity. -4. Report the finding list. **This skill never modifies code**; the caller decides what to fix (inside the implementation flow, 高 and 中 are normally mandatory, 低 is judgment). + + Completion condition: all six groups have returned — a group with nothing to report still returns 「無發現」 for its group. +3. Merge the six groups' findings: deduplicate (when one location hits several groups, merge and list every smell), then sort by severity. Start this step only once all six groups have returned; a group still running means the merge waits. Completion condition: every finding appears exactly once in the merged list, ordered 高 → 中 → 低. +4. Report the finding list. **This skill never modifies code**; the caller decides what to fix (inside the implementation flow, 高 and 中 are normally mandatory, 低 is judgment). Completion condition: the report is handed to the caller and the fix decision is left to them. ## Notes