Metadata

Skill ID

fbb6f62d-947e-43ff-aea7-64b81d7fb0fe

Version

1

Owner

387274b7-2891-478b-81b8-e11d5adb9319

Tags

gitpull-requestgithubcode-reviewcollaboration

Signature

verified

Integrity

OK

Content hash

f7e99298246e208bfeda365f3132fc44dc6829c52630ec5077a155fa91d5de62

Created

2026-08-15T05:27:03Z

Skill file

# PR Lifecycle

**Use when** creating or managing a pull request, from branch naming through merge. A well-run PR is small, self-explanatory, and easy to review.

## Branch Naming

```text
<type>/<short-description>
feat/add-bulk-cancel
fix/null-user-crash
refactor/extract-tokenizer
chore/update-deps
docs/readme-setup
```

```bash
git checkout -b feat/add-bulk-cancel
git push -u origin feat/add-bulk-cancel
```

### The Draft → Ready Transition

| State | When | What it signals |
|-------|------|-----------------|
| **Draft** | Work in progress, incomplete, or known-broken | "Don't review yet, still building" |
| **Ready** | Complete, self-reviewed, tests pass | "Please review this" |

```bash
# Open as draft (GitHub CLI)
gh pr create --draft --title "feat: add bulk cancel" --body "..."

# Mark ready when done
gh pr ready <PR_NUMBER>
```

## The PR Description Template

Every PR needs: **What / Why / How to test / Risks / Linked issue**.

```markdown
## What
Adds bulk order cancellation: select multiple orders and cancel them in one action.

## Why
Support receives hundreds of "cancel all my orders" requests. Bulk cancel
reduces manual clicks from 5 to 1 per order.

## How to test
1. Checkout `feat/add-bulk-cancel`
2. Run `pytest tests/orders/test_bulk_cancel.py -v`
3. Manually: select 3 orders → Cancel → confirm all 3 show "cancelled"

## Risks
- Batch operation could hit rate limits (>100 orders)
- Partial failure: what if 2 of 3 cancel but the 3rd fails?

## Linked issue
Closes #1234
```

## The Review Loop

```text
1. Open PR (draft → ready)
2. Request review (gh pr ready + mention reviewers)
3. Address comments — reply or fix, then re-request review
4. Keep the diff small and focused
5. Re-request review after changes
```

### Responding to comments

```bash
# After addressing review comments, push and re-request:
git commit -am "address review: handle partial failure case"
git push
gh pr ready <PR_NUMBER>  # if still draft
# Re-request review in the UI or via gh (some providers):
gh pr comment <PR_NUMBER> --body "@reviewer addressed all comments, ready for re-review"
```

### Force-pushing after review

```text
If you rebase/force-push after a review, EXPLAIN what changed.
Otherwise the reviewer has to re-read the entire diff.
```

```bash
git rebase origin/main
git push --force-with-lease
# Then comment: "Rebased onto main; only changed the merge base, no logic changes"
```

## Merge Strategy Selection

| Strategy | When | Result |
|----------|------|--------|
| **Squash** | Many small "wip" commits, or you want one clean commit | One commit per PR |
| **Rebase** | You want a linear history, each commit preserved | Commits replayed on main |
| **Merge commit** | Long-lived feature branches, you want the branch boundary | Merge commit + all commits |

```bash
# Squash merge (most common for feature work)
gh pr merge <PR_NUMBER> --squash

# Rebase merge (preserve individual commits)
gh pr merge <PR_NUMBER> --rebase

# Merge commit
gh pr merge <PR_NUMBER> --merge
```

### Decision rule

```text
Feature work with messy history -> SQUASH
Linear-history team, clean commits -> REBASE
Long-running branch / release branch -> MERGE COMMIT
```

## Guardrails

- **No PR without a description.** "See diff" is not a description — the reviewer can't know intent.
- **Keep diffs small** (<400 lines ideally). Large PRs get rubber-stamped or ignored.
- **Don't force-push after review without explaining.** You'll invalidate the reviewer's work.
- **Self-review before requesting review.** Read your own diff; catch typos and leftovers.
- **Don't merge your own PR without review** (if your team requires review) — get sign-off first.

## Pitfalls

| Pitfall | Fix |
|---------|-----|
| Opening a PR with no description | Use the What/Why/Test/Risks/Issue template |
| 2000-line diff | Split into multiple smaller PRs |
| Force-pushing after review silently | Comment what changed, or avoid force-push |
| Draft PR never marked ready | Set a reminder; don't leave it dangling |
| Merging without resolving CI | Wait for green CI, or explicitly note why it's red |

## Verify / Checklist

- [ ] Branch name follows `<type>/<description>` convention
- [ ] PR description has What/Why/Test/Risks/Linked issue
- [ ] CI is green (or red with explicit justification)
- [ ] Diff is small and focused (<400 lines)
- [ ] Self-reviewed before requesting review
- [ ] Review comments addressed and re-request sent
- [ ] Merge strategy chosen intentionally (squash/rebase/merge)

Attached files

No attached files.