event4u-app requesting-code-review
When to use
- About to run
/create-pror/prepare-for-review - A feature or bug fix is code-complete and the next step is "get eyes on it"
- A stacked PR is ready and you need the parent branch reviewer to context-switch smoothly
- Asking a human for a quick sanity check on a specific commit or diff
Do NOT use when:
- You are processing review feedback — use
receiving-code-review - The branch is not yet code-complete — the review-request gate requires green tests and a clean diff
- The change is documentation-only and has no behavior impact
Goal
Give the reviewer exactly the context they need — what changed, why, how to verify — without forcing them to reconstruct your thought process. A well-framed review request halves review time and reduces back-and-forth on missing context.
The Iron Law
NEVER REQUEST REVIEW FROM A BRANCH YOU HAVE NOT REVIEWED YOURSELF.
Self-review is the single cheapest filter. It catches the issues a human reviewer would flag in round one, so the human reviewer can spend time on the issues only they can see.
Procedure
1. Self-review first
Before asking anyone else:
- Read the full diff (
git diff <base>...<head>), not just the files you remember touching - Check for accidental debug output, dead code, leftover
dd(),console.log, commented-out blocks - Check for secrets in diff — API keys, connection strings, tokens
- Check file-system side effects — generated files, lockfile churn,
IDE configs,
.envchanges - Run the linter + tests (see
verify-completion-evidence) - If you find issues → fix them, do not ship them and hope the reviewer flags them
Use the review-changes command
as the structured walk-through.
2. Establish the diff baseline
Determine the correct base commit:
# Simple branch from main
BASE=$(git merge-base HEAD main)
# Stacked PR — parent branch is base, not main
BASE=$(git merge-base HEAD <parent-branch>)
HEAD=$(git rev-parse HEAD)
The base matters for the reviewer's diff view — wrong base = they review 80 unrelated commits.
3. Write the review request context
Any review request must answer four questions. If any is missing, the reviewer will ask — and that round trip is preventable.
| Question | Where it lives |
|---|---|
| What changed? | PR title (imperative, Conventional Commits) + summary bullets |
| Why? | Link to ticket / issue / Sentry event / user message |
| How do I verify it? | Test plan: commands to run, URLs to hit, expected behavior |
| What should I look at first? | Highlights: "pay attention to X because Y" or "skip Z, it is generated" |
See create-pr-description
for the full structured template, and
conventional-commits-writing
for the title format.
4. Keep the PR reviewable in size
- Target < 400 lines of real diff (excluding generated / lockfiles)
- If bigger — consider splitting into a stack (refactor PR → feature PR) so reviewers can handle each in one sitting
- Flag generated files explicitly in the description so reviewers skip them
- Never mix a refactor + behavior change in the same PR — reviewers cannot isolate the risk
5. Pick the right reviewer set
- Architectural impact → the code owner for the affected area
- Security-sensitive → a security-reviewer role if the project has one
- Bots → let Copilot / Greptile / Augment run automatically; do not gate human review on bot completion
- Cross-team change → each affected team's owner
If the project has a CODEOWNERS file, GitHub handles this
automatically — do not override without a reason.
6. Send and wait — do not nudge early
After the PR is open:
- Respond to questions, not to the implicit "where is my review?" schedule
- If the review is blocking and overdue → a single short nudge is appropriate; do not re-open or force-push to bump the PR list
When review comments arrive → switch to
receiving-code-review.
Output format
When handing the review request to the reviewer (PR body, Slack, email):
- Title — Conventional Commit imperative
- Summary — 2–5 bullets of what changed
- Motivation — why, with ticket / Sentry / user-message link
- Test plan — exact commands or URLs + expected result
- Risk / scope notes — what the reviewer should pay attention to
- Non-goals — what this PR deliberately does not do
- Screenshots / logs — when UI or runtime behavior changed
Gotchas
- "Ready for review" on a red CI run signals carelessness — wait for green or explain the specific failure
- A 1000-line PR with "no behavior change" still needs review — the reviewer has no way to confirm "no behavior change" without reading every line
- Auto-merge on approval bypasses re-review after later force-pushes — use deliberately, not by habit
- A PR description that says "see the code" is not a description — reviewers need the why
- Requesting review from someone without context (new hire, other team) without a longer pairing — they cannot do a deep review cold
- Rebasing during review invalidates line-comments; let reviewers finish a round before rebasing when possible
Do NOT
- Do NOT request review before self-review
- Do NOT request review on a branch with failing CI (except when explicitly documenting "CI broken, please eyeball X only")
- Do NOT mix unrelated changes in one PR
- Do NOT force-push during active review without a good reason and a note to the reviewer
- Do NOT leave the PR description empty
- Do NOT request review from reviewers who lack context just to get approvals
Anti-patterns
- "Small cleanup" titles on 800-line diffs
- Test plan that says "should work"
- Hiding a risky change inside a 40-file refactor PR
- Pinging reviewers before CI goes green
- Requesting review as a substitute for thinking the problem through
When to hand over to another skill / command
- Self-review walkthrough →
review-changes - Bringing a stacked branch up to date first →
prepare-for-review - Writing the PR description →
create-pr-description - Actually opening the PR →
create-pr - Writing the commits themselves →
commit,conventional-commits-writing - Verifying readiness →
verify-completion-evidence - Processing the feedback when it arrives →
receiving-code-review
Validation checklist
Before asking for review:
- Self-review walkthrough done on the full diff
- Linter + targeted tests + full test suite green
- PR title follows Conventional Commits
- PR description has summary, motivation, test plan, risk notes
- Diff is reviewable in size (split if > 400 lines of real code)
- CODEOWNERS / reviewer set is correct
- No debug code, secrets, or stray files in the diff
- No unrelated changes bundled in