Skip to content

Commit fca1df9

Browse files
committed
docs: add review restraint policy and summary table to pr-review skill
1 parent 0f90604 commit fca1df9

File tree

1 file changed

+30
-2
lines changed

1 file changed

+30
-2
lines changed

.github/skills/pr-review/SKILL.md

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,22 @@ description: Address review comments and CI failures for the current branch's PR
99
- Key commands: `gh pr list --head <BRANCH>` · `gh pr view <PR> --comments` · `gh pr checks <PR>` · `gh run view <RUN_ID> --log-failed`
1010
- Workflow fixes **cannot be verified locally** — the fix is only confirmed once the remote CI re-runs and passes
1111

12+
### Review Restraint Policy
13+
14+
Before acting on any review comment or suggestion, classify it by importance:
15+
16+
| Level | Criteria | Action |
17+
|-------|----------|--------|
18+
| **P1 — Must Fix** | Bug, security issue, broken behavior, API contract violation, CI failure | Fix immediately |
19+
| **P2 — Should Fix** | Correctness risk, meaningful maintainability improvement, clear code smell with real impact | Fix with brief justification |
20+
| **P3 — Optional** | Style preference, minor naming nitpick, debatable design choice, "could be cleaner" | **Do NOT auto-fix** — surface to user for decision |
21+
| **P4 — Reject** | Contradicts project conventions, introduces unnecessary complexity, or is factually wrong | Reject with explanation |
22+
23+
**Rules:**
24+
- **Do not implement P3 suggestions automatically.** Note them in the final summary and let the user decide.
25+
- **Do not add comments to code** unless the comment explains non-obvious logic that is truly necessary. Never add comments just to acknowledge a review suggestion was applied.
26+
- When in doubt about importance, prefer the lower severity (P3/P4) and defer to the user rather than making the change.
27+
1228
## Procedure
1329

1430
1. **Locate PR** — get PR number for current branch
@@ -22,12 +38,24 @@ description: Address review comments and CI failures for the current branch's PR
2238
5. If it **still fails** → fetch new logs (`gh run view <NEW_RUN_ID> --log-failed`) and repeat from step i
2339
- **Code issue, non-breaking**: fix source code directly
2440
- **Code issue, breaking change required**: stop and report to developer for a decision
25-
3. **Address review comments** — implement valid feedback; document why invalid ones are rejected
41+
3. **Address review comments** — apply the Review Restraint Policy to each comment:
42+
- P1/P2: implement the fix
43+
- P3: record in summary, do **not** modify code, ask user
44+
- P4: record rejection reason in summary
2645
4. **Commit & push** — single commit covering all non-workflow fixes (workflow fixes are pushed incrementally during step 2)
2746
5. **Final verification** — once all fixes are applied, confirm every check is green: `GH_PAGER= gh pr checks <PR>`
2847

2948
## Output
3049

3150
- Per CI failure: root cause and resolution (or escalation reason)
32-
- Per review comment: resolution or rejection reason
3351
- After all fixes: summary of check statuses confirming all green
52+
- **Review summary table** — produced at the end of every review session:
53+
54+
```
55+
| # | Source (comment / CI) | Issue description | Priority | Action taken | Reason if not fixed |
56+
|---|-----------------------|-------------------|----------|--------------|---------------------|
57+
| 1 | Reviewer @xxx | e.g. rename foo() | P3 | Not fixed | Style preference, no functional impact — deferred to user |
58+
| 2 | CI: lint | e.g. null-check | P1 | Fixed | — |
59+
```
60+
61+
All P3/P4 items that were **not** fixed must appear in this table with a clear reason, so the user can make an informed decision.

0 commit comments

Comments
 (0)