# Code Review

> Comprehensive code review covering Functionality, Security (OWASP), Performance, and Maintainability. Includes good/bad examples.

- Skill: `krishnakanthb13/code-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add krishnakanthb13/code-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/krishnakanthb13/code-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Security
- Author: krishnakanthb13 (https://skillmd.com/u/krishnakanthb13)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/krishnakanthb13/code-review

---


# Code Review Checklist

Use this skill to conduct a thorough, senior-level code review.

## Inputs
*   **Target**: The file or directory to review.
*   **Focus**: (Optional) Specific area to check (e.g., security, performance).

## Tooling Strategy
*   Use `view_file` to read the code.
*   Use `grep_search` if looking for specific patterns across multiple files.

## Workflow

### 1. Functionality & Logic (The "Does it Work?" Check)
*   **Requirements**: Does this solving the stated problem?
*   **Edge Cases**: Nulls, empty states, limits (min/max).
*   **Error Handling**: Are errors caught, logged, and handled gracefully (no silent failures)?

### 2. Security (The OWASP Check)
*   **Injection**: Check for raw SQL, `innerHTML`, or `eval()`.
    *   *❌ Bad*: `db.query("SELECT * FROM users WHERE name = " + input)`
    *   *✅ Good*: `db.query("SELECT * FROM users WHERE name = $1", [input])`
*   **Secrets**: Ensure NO API keys or passwords are committed. (Use `.env`).
*   **Auth**: Are permission checks present on sensitive routes?

### 3. Performance (The Scalability Check)
*   **Loops**: No expensive operations inside loops (e.g., DB calls).
*   **N+1 Problems**: Check for fetching data in a loop instead of batches.
*   **Memory**: Are listeners/connections cleaned up?

4. **Code Quality (The Maintainability Check)**
*   **Naming**: Variables should be intentional (`isValid` vs `v`).
*   **DRY (Don't Repeat Yourself)**: Can distinct logic be extracted to a helper?
*   **SOLID Principles**:
    *   **S**: Does the class/function do only ONE thing?
    *   **O**: Can we add features without changing this code?
    *   **L**: Can subclasses be used interchangeably?
    *   **I**: Are interfaces too bloated?
    *   **D**: Is it hard-coded to a specific implementation? (Use dependency injection).
*   **Comments**: Explain *WHY*, not *WHAT*.

### 5. Git & Hygiene
*   **Commits**: Are they atomic and descriptive?
*   **Leftovers**: No `console.log` (debug only), commented-out code, or TODOs without an issue tracking them.

## Output Format
Structure your review like this:

**Summary**: [High level opinion - Ready to Merge / Changes Requested]

| Category | Status | Notes |
| :--- | :--- | :--- |
| **Functionality** | ✅ / ⚠️ | ... |
| **Security** | ✅ / 🔴 | ... |
| **Performance** | ✅ / ⚠️ | ... |

**Detailed Comments**:
*   [File/Line]: [Issue description]
    *   *Suggestion*: [Code snippet check]

## Comprehensive Review Guidelines

### 1. The Review Mindset
- **Goals**: Catch bugs, ensure maintainability, share knowledge, improve architecture.
- **Not Goals**: Nitpicking formatting, showing off, rewriting to personal preference.
- **Provide Effective Feedback**: Be specific, actionable, educational, and focused on the code, not the person. Suggest, don't command.

### 2. Differentiate Severity (The Label System)
Use explicit labels to set expectations:
- 🔴 `[blocking]`: Must fix before merge (e.g., security flaws, breaking bugs).
- 🟡 `[important]`: Should fix, discuss if disagree (e.g., architectural issues, N+1 queries).
- 🟢 `[nit]`: Nice to have, not blocking (e.g., minor naming improvements).
- 💡 `[suggestion]`: Alternative approach to consider.
- 📚 `[learning]`: Educational comment, no action needed.
- 🎉 `[praise]`: Acknowledge good work and solid patterns!

### 3. Review Techniques
- **The Question Approach**: Ask questions to encourage thinking (e.g., "What happens if `items` is empty?" instead of "This fails if empty").
- **Collaborative Language**: Use suggestions (e.g., "Would it make sense to extract this?" instead of "Extract this").
- **Avoid Implementation Detail Reviews**: Ensure tests describe behavior, not internal state (e.g., test what happens on click, not that an internal counter incremented).

### 4. Handling Disagreements
If there's disagreement: Seek to understand the author's approach, acknowledge valid points, request data (like a benchmark) if needed, and know when to let go if it's working and not critical.