mirror of
https://github.com/penpot/penpot.git
synced 2026-08-23 21:28:38 +00:00
♻️ Rename code-review-and-quality skill to code-review
Rename skill directory and update all references to follow the same naming pattern as plan-review. Simplify review.md command from 138 to 25 lines — remove redundant content that duplicated what the skills already define. The command now acts as a thin router; the skills own the methodology. AI-assisted-by: mimo-v2.5-pro
This commit is contained in:
parent
7c85837290
commit
0cacf9bd99
@ -3,60 +3,14 @@ Act as a senior software engineer and perform a thorough review.
|
||||
## Instructions
|
||||
|
||||
1. **Determine what is being reviewed** from the provided context:
|
||||
- **If it is a plan** (implementation plan, design document, task breakdown) → follow the **Plan Review** path below.
|
||||
- **If it is code** (diff, PR, code change) → follow the **Code Review** path below.
|
||||
- **If it is a plan** (implementation plan, design document, task breakdown) → load the **`plan-review`** skill.
|
||||
- **If it is code** (diff, PR, code change) → load the **`code-review`** skill.
|
||||
|
||||
---
|
||||
2. Read `AGENTS.md` and follow its instructions for finding and reading all related testing documentation from memories before reviewing.
|
||||
|
||||
## Code Review Path
|
||||
3. **Skip generated files, lockfile-only changes, and unrelated modifications** unless they introduce security risks.
|
||||
|
||||
1. Load the **`code-review-and-quality`** skill — it defines the five axes, core principles (DRY, KISS, YAGNI), severity taxonomy, and output format.
|
||||
2. Read `AGENTS.md` and follow its instructions for finding and reading all related testing documentation from memories before reviewing the code.
|
||||
3. Determine the diff or code to review from the provided context.
|
||||
4. **Skip generated files, lockfile-only changes, and unrelated modifications** unless they introduce security risks.
|
||||
5. Read the diff and the surrounding context for each changed file.
|
||||
6. Review across all five axes: correctness, readability, architecture, security, performance.
|
||||
7. Produce the review using the **Code Review Format** below.
|
||||
8. For each finding:
|
||||
- State the severity (Critical / High / Medium / Low / Suggestion)
|
||||
- Identify the file and line
|
||||
- Describe failure circumstances
|
||||
- **For Critical/High**: Provide a concrete fix with a code snippet showing the corrected code
|
||||
- **For Medium/Low**: Describe the fix clearly; code snippet optional
|
||||
- If multiple approaches exist, briefly note trade-offs
|
||||
9. **Perform a second review pass if the change is complex:**
|
||||
- **Complex indicators**: Critical/High findings, multiple files (>5), architectural changes, security-sensitive code, >300 lines changed
|
||||
- **Skip for simple changes**: Typo fixes, formatting, small bug fixes (<50 lines), single-file changes with no findings
|
||||
- Second pass checks:
|
||||
- Validate severity assignments: Are Critical/High findings truly blockers?
|
||||
- Catch missed issues: Edge cases, error paths, test gaps overlooked in first pass
|
||||
- Remove false positives: Discard findings that aren't real issues
|
||||
- Verify fixes: Are the proposed solutions actually correct and complete?
|
||||
|
||||
---
|
||||
|
||||
## Plan Review Path
|
||||
|
||||
1. Load the **`plan-review`** skill — it defines the six axes, severity taxonomy, and output format.
|
||||
2. Read the full plan from the provided context.
|
||||
3. Review across all six axes: completeness, task quality, architecture & sequencing, risk coverage, actionability, and proposed code quality (if the plan includes implementation details).
|
||||
4. Produce the review using the **Plan Review Format** below.
|
||||
5. For each finding:
|
||||
- State the severity (Critical / Required / Nit / Optional / FYI)
|
||||
- Identify the section or task it refers to
|
||||
- Describe the gap or problem
|
||||
- **For Critical/Required**: Propose a concrete fix or addition
|
||||
- **For Nit/Optional**: Describe the improvement; concrete text optional
|
||||
6. **Perform a second review pass if the plan is complex:**
|
||||
- **Complex indicators**: Critical findings, >10 tasks, migrations or breaking changes, security-sensitive features
|
||||
- **Skip for simple plans**: 1–2 tasks, no risks, no code proposals
|
||||
- Second pass checks:
|
||||
- Validate severity assignments
|
||||
- Catch missed gaps: edge cases, missing dependencies, unaddressed risks
|
||||
- Remove false positives
|
||||
- Verify proposed remedies are actionable
|
||||
|
||||
---
|
||||
4. Follow the loaded skill's process and produce its output format.
|
||||
|
||||
## Strong Rules
|
||||
|
||||
@ -64,75 +18,8 @@ Act as a senior software engineer and perform a thorough review.
|
||||
2. Do not modify any code and do not create a commit — this command only reviews.
|
||||
3. Be specific and constructive. "This could be better" is not helpful — explain why and how.
|
||||
4. Prioritize by impact. One structural issue outweighs ten nits.
|
||||
5. Missing tests are an issue, not a suggestion. If tests are missing or inadequate for new functionality, report it as a severity-tagged finding in the findings sections below — High severity (code) or Required (plan) — never as a recommendation.
|
||||
5. Missing tests are an issue, not a suggestion. Report as a severity-tagged finding — never as a recommendation.
|
||||
|
||||
## Context
|
||||
|
||||
$ARGUMENTS
|
||||
|
||||
## Expected Format — Code Review
|
||||
|
||||
```
|
||||
## Review Summary
|
||||
[1-2 sentences on what the change does and overall assessment]
|
||||
|
||||
## Critical/High Findings
|
||||
|
||||
### [Severity] file.ts:123
|
||||
**Issue**: [Description of the problem]
|
||||
**Impact**: [What could go wrong if this is not fixed]
|
||||
**Fix**:
|
||||
|
||||
````[language]
|
||||
// Current code
|
||||
[problematic code]
|
||||
|
||||
// Fixed code
|
||||
[corrected code]
|
||||
[Optional: note trade-offs if multiple approaches exist]
|
||||
````
|
||||
|
||||
### [Severity] file.ts:456
|
||||
**Issue**: [Description of the problem]
|
||||
**Impact**: [What could go wrong if this is not fixed]
|
||||
**Fix**: [Clear description of the fix; code snippet if it clarifies]
|
||||
|
||||
## Other Findings
|
||||
|
||||
### [Severity] file.ts:789
|
||||
**Issue**: [Description]
|
||||
**Impact**: [Minor consequence or risk]
|
||||
**Fix**: [Clear description; code snippet optional]
|
||||
|
||||
## Positive Observations
|
||||
[2-3 specific things done well]
|
||||
|
||||
## Verdict
|
||||
[Approve / Request Changes / Needs Discussion]
|
||||
[If Request Changes: list the must-fix items]
|
||||
```
|
||||
|
||||
## Expected Format — Plan Review
|
||||
|
||||
```
|
||||
## Review Summary
|
||||
[1-2 sentences on the plan's goal and overall assessment]
|
||||
|
||||
## Critical/Required Findings
|
||||
### [Severity] [Section or Task N]
|
||||
**Issue**: [Description of the gap or problem]
|
||||
**Impact**: [What could go wrong during implementation]
|
||||
**Proposed fix**: [Concrete addition or change to the plan]
|
||||
|
||||
## Other Findings
|
||||
### [Severity] [Section or Task N]
|
||||
**Issue**: [Description]
|
||||
**Proposed fix**: [Clear description; concrete text optional]
|
||||
|
||||
## Strengths
|
||||
[2-3 specific things done well in the plan]
|
||||
|
||||
## Verdict
|
||||
[Approve / Request Changes / Needs Discussion]
|
||||
[If Request Changes: list the must-fix items]
|
||||
```
|
||||
|
||||
@ -1,5 +1,5 @@
|
||||
---
|
||||
name: code-review-and-quality
|
||||
name: code-review
|
||||
description: Conducts multi-axis code review. Use before merging any change. Use when reviewing code written by yourself, another agent, or a human. Use when you need to assess code quality across multiple dimensions before it enters the main branch.
|
||||
---
|
||||
|
||||
@ -87,7 +87,7 @@ Can an implementer actually execute this?
|
||||
|
||||
### 6. Proposed Code Quality *(when the plan includes implementation details)*
|
||||
|
||||
If the plan proposes code shapes, function signatures, data structures, or API designs, evaluate those proposals against `code-review-and-quality` criteria:
|
||||
If the plan proposes code shapes, function signatures, data structures, or API designs, evaluate those proposals against `code-review` criteria:
|
||||
|
||||
- **Correctness:** Do the proposed types/signatures handle edge cases (null, empty, boundaries)?
|
||||
- **Readability:** Are proposed names descriptive and consistent with project conventions?
|
||||
@ -215,7 +215,7 @@ Check that the plan can actually confirm it worked:
|
||||
If the plan includes code snippets, types, or API designs:
|
||||
|
||||
```
|
||||
- Load code-review-and-quality skill for criteria
|
||||
- Load code-review skill for criteria
|
||||
- Check proposed signatures for edge cases
|
||||
- Verify naming follows project conventions
|
||||
- Confirm abstractions follow existing patterns
|
||||
@ -310,6 +310,6 @@ If the plan includes code snippets, types, or API designs:
|
||||
## See Also
|
||||
|
||||
- For producing plans, use the `planner` skill
|
||||
- For reviewing implemented code, use `code-review-and-quality` — also the criteria source for axis 6
|
||||
- For reviewing implemented code, use `code-review` — also the criteria source for axis 6
|
||||
- For security-specific concerns, see `security-and-hardening`
|
||||
- For testing strategy guidance, see `testing`
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user