DEV Community

Tarun Moorjani
Tarun Moorjani

Posted on

Writing PR Descriptions That Get Approved Faster

I've reviewed thousands of PRs. Some get approved in 10 minutes. Others sit for days.

The difference isn't code quality. It's the PR description.

Bad PR descriptions look like this:

Title: Fix bug

Description: Fixed the thing
Enter fullscreen mode Exit fullscreen mode

I have to:

  • Clone the PR
  • Read every line of code
  • Figure out what changed
  • Guess why it changed
  • Hunt for the related issue
  • Ask questions in comments
  • Wait for answers
  • Re-review

Time to approve: 2-3 days, multiple review cycles

Good PR descriptions look like this:

Title: Fix user profile crash when email is null

## Problem
Users with null emails (legacy data from 2019 migration) crash the profile 
page when clicking "Edit Profile". Error: "Cannot read property 'toLowerCase' 
of null"

Affects ~500 users based on database query.

## Solution
Added null check before calling toLowerCase(). If email is null, show 
"Add email" prompt instead.

## Testing
- [x] Manual: Tested with user ID 12345 (has null email)
- [x] Unit: Added test for null email case
- [x] Verified existing tests still pass

## Screenshots
[Before] - Crash screen
[After] - "Add email" prompt

Fixes #1234
Enter fullscreen mode Exit fullscreen mode

I can:

  • Understand the problem immediately
  • See the solution makes sense
  • Trust it's been tested
  • Approve in 5 minutes

Time to approve: 5-10 minutes, one review cycle

Let me show you how to write PR descriptions that get approved fast.

The 5-Minute Rule

If a reviewer can't understand your PR in 5 minutes, you've failed.

Your goal: Make reviewing effortless.

Every second a reviewer spends confused is a second they might:

  • Ask questions (delays approval)
  • Request changes (delays approval)
  • Ignore your PR (delays approval)

Your job: Answer all their questions before they ask.

The PR Description Template

Copy this. Customize it. Use it every time.

## Problem

[What bug are you fixing? What feature are you adding? Why does this matter?]

[Impact: How many users? How severe? Business context?]

## Solution

[High-level approach. NOT line-by-line code explanation.]

[Key decisions made and why]

[Any alternatives considered and why rejected]

## Testing

- [ ] Manual testing steps performed
- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [ ] Tested edge cases: [list them]

## Risks

[Any concerns? Known limitations? Future work needed?]

## Screenshots/Videos

[If UI change: before/after screenshots or screen recording]

## Related

Fixes #[issue number]
Related to #[other PR or issue]
Depends on #[blocking PR]

---

## Reviewer Notes

[Anything specific reviewers should focus on? Anything you're unsure about?]
Enter fullscreen mode Exit fullscreen mode

Section-by-Section Breakdown

Title: Make It Searchable

Bad titles:

Fix bug
Update component
Changes
WIP: stuff
Enter fullscreen mode Exit fullscreen mode

Good titles:

Fix profile crash when user email is null
Add pagination to user management table
Upgrade React Router from v5 to v6
Refactor authentication to use OAuth2
Enter fullscreen mode Exit fullscreen mode

Template:

[Action] [What] [Where/When/Why]
Enter fullscreen mode Exit fullscreen mode

Examples:

Fix infinite loop in useEffect when deps change rapidly
Add loading state to checkout form submit button
Remove deprecated props from Button component
Upgrade TypeScript from 4.9 to 5.3
Enter fullscreen mode Exit fullscreen mode

Why it matters: Reviewers often search for PRs. "Fix bug" is unsearchable. "Fix profile crash when email is null" is specific.

Problem: Why Does This PR Exist?

This is the most important section. If reviewers don't understand the problem, they can't evaluate the solution.

Bad:

## Problem
Need to fix the profile page
Enter fullscreen mode Exit fullscreen mode

Good:

## Problem

Users with null emails (legacy data from 2019 migration) crash when clicking 
"Edit Profile" button. 

Error: `Cannot read property 'toLowerCase' of null` at ProfileForm.tsx:45

**Impact:**
- Affects ~500 users (0.5% of active users)
- Support tickets: 12 in last week
- Critical path: Users can't update their profiles
- Business impact: Users can't change their notification preferences

**Root cause:**
The 2019 migration from old auth system didn't require emails. Current code 
assumes all users have emails.
Enter fullscreen mode Exit fullscreen mode

What to include:

  • Exact error message (if it's a bug)
  • Impact: how many users, how severe
  • Business context: why this matters
  • Root cause: what's actually wrong

Bad example - vague:

## Problem
The form is broken
Enter fullscreen mode Exit fullscreen mode

Good example - specific:

## Problem

Checkout form doesn't validate credit card expiration dates, allowing users to 
submit expired cards.

**Impact:**
- Payment processor rejects 15% of transactions
- Users see generic "Payment failed" error
- Support receives ~50 tickets/day asking why payment failed
- Lost revenue: ~$50k/month in abandoned carts

**Root cause:**
Validation regex only checks format (MM/YY) but doesn't compare to current date.
Enter fullscreen mode Exit fullscreen mode

Solution: What Did You Do?

Explain your approach at a high level. Don't describe every line of code.

Bad:

## Solution
Added a check
Enter fullscreen mode Exit fullscreen mode

Bad (too detailed):

## Solution
On line 45, I added an if statement that checks if email is null. If it is, 
I set a variable called showAddEmail to true. Then on line 67, I check if 
showAddEmail is true, and if so, I render a div with className "add-email-prompt" 
that contains a button...
Enter fullscreen mode Exit fullscreen mode

Good:

## Solution

Added null check before processing email:
1. Check if user.email is null
2. If null, show "Add email" prompt instead of edit form
3. Prompt includes link to account settings to add email

**Key decision:** 
Chose to show prompt rather than pre-populate with empty string, because empty 
string would allow users to save profile without fixing the root issue.

**Alternative considered:** 
Backfill all null emails with "noemail@example.com". Rejected because this 
creates invalid data and doesn't solve the problem for future users.
Enter fullscreen mode Exit fullscreen mode

What to include:

  • High-level approach (numbered steps work well)
  • Key decisions and reasoning
  • Alternatives considered (shows you thought it through)

Pro tip: If you considered multiple approaches, show them:

## Solution

**Approach taken:** Add validation to frontend form

**Why not these alternatives:**
1. ❌ Fix in backend - Requires API change, affects mobile apps
2. ❌ Backfill null emails - Creates invalid data
3. ❌ Disable form for null emails - Poor UX, users can't update anything
Enter fullscreen mode Exit fullscreen mode

Testing: How Do I Know It Works?

Show you've tested thoroughly. Reviewers trust tested code.

Bad:

## Testing
Tested it, works fine
Enter fullscreen mode Exit fullscreen mode

Good:

## Testing

**Manual testing:**
- [x] Tested with user ID 12345 (has null email) - shows prompt correctly
- [x] Tested with user ID 67890 (has email) - edit form works normally
- [x] Clicked "Add email" link - navigates to settings correctly
- [x] Added email in settings, returned to profile - edit form now appears

**Automated tests:**
- [x] Added unit test: `ProfileForm.test.tsx` - null email case
- [x] Added unit test: `ProfileForm.test.tsx` - valid email case
- [x] Updated integration test: `profile-flow.test.tsx`
- [x] All existing tests passing (142/142)

**Edge cases tested:**
- [x] Email is empty string (not just null)
- [x] Email is whitespace only
- [x] User spams "Add email" button (no duplicate navigations)

**Browser testing:**
- [x] Chrome 120
- [x] Safari 17
- [x] Firefox 121
- [x] Mobile Safari iOS 17
Enter fullscreen mode Exit fullscreen mode

Checklist template:

## Testing

**Functionality:**
- [ ] Happy path works
- [ ] Error cases handled
- [ ] Edge cases tested

**Tests added:**
- [ ] Unit tests for new logic
- [ ] Integration tests for user flow
- [ ] All existing tests still pass

**Manual verification:**
- [ ] Tested locally
- [ ] Tested in staging
- [ ] Verified fix with reproduction steps from bug report
Enter fullscreen mode Exit fullscreen mode

Risks: What Could Go Wrong?

Be honest about risks. Reviewers appreciate transparency.

Bad:

## Risks
None
Enter fullscreen mode Exit fullscreen mode

Good:

## Risks

**Known limitations:**
- This fixes the immediate crash, but doesn't solve the root problem of users 
  having null emails. Follow-up PR needed to prompt users to add emails.

**Performance impact:**
- Added database query to check email status. Query is fast (~5ms) and cached, 
  but monitoring recommended.

**Rollback plan:**
- If issues arise, can disable via feature flag `PROFILE_NULL_EMAIL_CHECK`
- No database migrations, safe to rollback

**Future work needed:**
- #1235 - Backfill null emails (non-urgent)
- #1236 - Add email requirement to onboarding flow
Enter fullscreen mode Exit fullscreen mode

What to include:

  • Known limitations
  • Performance implications
  • Rollback strategy
  • Future work needed

Screenshots/Videos: Show, Don't Tell

For UI changes, screenshots are mandatory.

Template:

## Screenshots

**Before:**
![Before](url-to-image)
[Description of problem]

**After:**
![After](url-to-image)
[Description of fix]

**Screen recording:**
[Link to Loom/video showing interaction]
Enter fullscreen mode Exit fullscreen mode

Pro tips:

  • Use side-by-side before/after when possible
  • Annotate screenshots (arrows, circles) to highlight changes
  • For interactions, use screen recordings (Loom, CloudApp)
  • For responsive changes, show mobile + desktop

Example:

## Screenshots

**Before:** Empty email crashes the page
![crash](https://i.imgur.com/crash.png)

**After:** Shows "Add email" prompt instead
![fixed](https://i.imgur.com/fixed.png)

**Mobile view:**
![mobile](https://i.imgur.com/mobile.png)

**Demo video:** https://www.loom.com/share/abc123
Shows full flow: crash → fix → adding email → form appearing
Enter fullscreen mode Exit fullscreen mode

Related: Link Everything

Connect your PR to the broader context.

Template:

## Related

Fixes #1234
Closes #1235
Resolves #1236

Part of #1240 (epic)
Depends on #1238 (must merge first)
Blocks #1242 (waiting on this)
Related to #1237 (similar issue)

[Design spec](link-to-figma)
[Tech spec](link-to-doc)
[Slack discussion](link-to-thread)
Enter fullscreen mode Exit fullscreen mode

GitHub keywords that auto-close issues:

  • Fixes #123
  • Closes #123
  • Resolves #123

Why link everything:

  • Reviewers see full context
  • Issues auto-close when PR merges
  • Future developers understand the history
  • Stakeholders can track progress

Reviewer Notes: Direct Attention

Tell reviewers what to focus on and what to ignore.

Template:

## Reviewer Notes

**Focus areas:**
- `ProfileForm.tsx` - Main logic change
- `profile.test.tsx` - New test coverage

**Can skip:**
- `package-lock.json` - Dependency update (unrelated)
- `translations/` - Auto-generated from i18n tool

**Questions for reviewers:**
- Line 45: Is this error message clear enough?
- Line 67: Should we log this to analytics?

**Known issues (will fix before merge):**
- [ ] TODO on line 123 - will implement before merging
- [ ] Commented-out code - will remove
Enter fullscreen mode Exit fullscreen mode

Why this helps:

  • Saves reviewers time
  • Gets feedback on specific concerns
  • Shows you're thoughtful about the review

Size Matters: Keep PRs Small

Bad: 47 files changed, 3,000 lines added

Reviewers see this and:

  • Put it off until "later"
  • Skim instead of reviewing carefully
  • Miss bugs

Good: 3 files changed, 150 lines added

Reviewers can:

  • Review in one sitting
  • Understand all changes
  • Provide thoughtful feedback

Guidelines:

  • Ideal: < 200 lines changed
  • OK: 200-400 lines changed
  • Large: 400-800 lines changed (split if possible)
  • Too large: > 800 lines changed (definitely split)

How to split large PRs:

Epic: Implement user profile redesign

PR 1: Add database schema for profile fields (#1234)
PR 2: Create ProfileCard component (#1235) ← You are here
PR 3: Integrate ProfileCard into profile page (#1236)
PR 4: Add profile editing flow (#1237)
PR 5: Migrate existing users to new schema (#1238)
Enter fullscreen mode Exit fullscreen mode

Each PR:

  • Is independently reviewable
  • Can be merged separately
  • Builds on previous PRs

In your PR description: ""

## Context

This is part 2 of 5 in the profile redesign epic (#1233).

**Previous PRs:**
- [x] #1234 - Database schema (merged)

**This PR:**
- Creates reusable ProfileCard component
- Adds unit tests
- Adds Storybook stories

**Next PRs:**
- [ ] #1236 - Integration into profile page
- [ ] #1237 - Editing flow
- [ ] #1238 - User migration
Enter fullscreen mode Exit fullscreen mode

Real Examples: Bad vs. Good

Example 1: Bug Fix

❌ Bad PR:

Title: fix

fixed the bug where the thing broke
Enter fullscreen mode Exit fullscreen mode

Reviewer questions:

  • What bug?
  • What thing?
  • How did it break?
  • How did you fix it?
  • How do I test it?
  • Is this related to any issue?

✅ Good PR:

Title: Fix duplicate charges when user double-clicks "Pay Now"

## Problem

Users who double-click the "Pay Now" button get charged twice. 

**Impact:**
- 23 duplicate charges in the last week
- $4,500 in refunds processed
- 15 angry support tickets
- Payment processor flagging us for suspicious activity

**Root cause:**
No debouncing on submit button. Each click triggers a separate API call.

## Solution

1. Disable button immediately on first click
2. Re-enable only after API response (success or error)
3. Add 1-second debounce as secondary protection

**Why not just debounce:**
Debouncing alone isn't enough. If API is slow, users might click again after 
1 second. Disabling the button is more reliable.

## Testing

**Manual:**
- [x] Single click - works normally
- [x] Double click - only one charge created
- [x] Spam clicking - only one charge created
- [x] Slow network (throttled to 3G) - still works correctly

**Automated:**
- [x] Added test: double-click prevention
- [x] Added test: button re-enables on error
- [x] All existing payment tests passing (24/24)

**Verified in production:**
- [x] Tested in staging with production-like load
- [x] Checked Datadog for error rate (0% increase)

## Risks

**Low risk change:**
- Only affects button state, no payment logic changes
- Can rollback via feature flag `PAYMENT_BUTTON_DEBOUNCE`
- If button gets stuck disabled, users can refresh page

## Screenshots

**Before:** Button stays enabled, multiple clicks allowed
![before](https://i.imgur.com/before.png)

**After:** Button disables on first click, re-enables after response
![after](https://i.imgur.com/after.png)

**Demo:** https://www.loom.com/share/abc123
Shows spam clicking, only one charge created

Fixes #1234
Enter fullscreen mode Exit fullscreen mode

Example 2: New Feature

❌ Bad PR:

Title: Add pagination

Added pagination to the table
Enter fullscreen mode Exit fullscreen mode

✅ Good PR:

Title: Add pagination to user management table (1000+ users)

## Problem

User management page loads all users at once, causing:
- 15+ second load time for admins with 1000+ users
- Browser memory issues (crashes on 2000+ users)
- Unable to find specific users quickly

**User feedback:**
"I have 1500 users and the page won't even load anymore" - Customer #4521

## Solution

Implement server-side pagination:
1. Load 50 users per page (configurable)
2. Add pagination controls (prev/next, page numbers)
3. Maintain filters and search across pages
4. Show total count and current range

**Technical approach:**
- Backend: Added `limit` and `offset` query params to `/api/users`
- Frontend: useQuery with pagination state
- Maintained backward compatibility (defaults to 50)

**Design decisions:**
- 50 users per page: balances load time vs. scrolling
- Show 5 page numbers at a time (... for rest)
- Preserve search/filters when changing pages

## Testing

**Manual:**
- [x] Tested with 10 users (shows 1 page)
- [x] Tested with 150 users (shows 3 pages)
- [x] Tested with 1500 users (page load < 2 seconds)
- [x] Search works across all pages
- [x] Filters work with pagination
- [x] URL updates with page number (shareable links)
- [x] Back button works correctly

**Automated:**
- [x] Unit tests: pagination logic
- [x] Unit tests: page number calculation
- [x] Integration test: navigating between pages
- [x] Integration test: filters + pagination
- [x] API tests: limit/offset params

**Performance:**
- Before: 15s load (1000 users)
- After: 1.8s load (50 users per page)
- Backend query time: 45ms (with indexes)

## Risks

**API changes:**
- Added optional query params (backward compatible)
- Old API calls (no params) still work, default to 50

**Future improvements needed:**
- #1245 - Add "items per page" selector
- #1246 - Infinite scroll option
- #1247 - Better mobile pagination UI

## Screenshots

**Before:** All 1000+ users in one long list
![before](https://i.imgur.com/before.png)

**After:** Clean pagination, 50 users per page
![after](https://i.imgur.com/after.png)

**Pagination controls:**
![controls](https://i.imgur.com/controls.png)

**Mobile view:**
![mobile](https://i.imgur.com/mobile.png)

## Related

Fixes #1234
Part of performance epic #1200
Design: https://www.figma.com/file/abc123
Enter fullscreen mode Exit fullscreen mode

Special Cases

Refactoring PRs

Title: Refactor authentication context to use zustand

## Why Refactor?

Current Context API implementation has issues:
- Causes unnecessary re-renders (entire app re-renders on auth change)
- Difficult to test (requires Provider wrapper)
- 200+ lines of boilerplate

## Changes

Replace Context API with zustand:
- ~200 lines removed
- Simpler API (`useAuth()` instead of `useContext(AuthContext)`)
- Better performance (selective subscriptions)
- Easier to test (no Provider needed)

## Migration Strategy

**Backward compatible:**
- Old Context API still works (deprecated)
- New zustand store available alongside
- Will remove Context in next PR after migration complete

**Migration path for other developers:**
Enter fullscreen mode Exit fullscreen mode


diff

  • const { user } = useContext(AuthContext);
  • const user = useAuth(state => state.user);

## Testing

- [x] All existing tests still pass (no changes needed)
- [x] Added zustand store tests
- [x] Verified auth flow works identically
- [x] No console errors in dev/prod

## Performance

**Before:**
- Login triggers 47 component re-renders
- DevTools shows deep re-render tree

**After:**
- Login triggers 3 component re-renders
- Only components using auth state re-render

Fixes #1234 (performance issue)
Enter fullscreen mode Exit fullscreen mode

Dependency Updates

Title: Upgrade React Router from v5 to v6

## Why Upgrade?

- v5 is deprecated (no security updates)
- v6 has better TypeScript support
- v6 has better hooks API
- Blocking upgrade to React 18

## Breaking Changes

**1. Route syntax changed:**
Enter fullscreen mode Exit fullscreen mode


diff

  • } />

**2. useHistory → useNavigate:**
Enter fullscreen mode Exit fullscreen mode


diff

  • const history = useHistory();
  • history.push('/home');
  • const navigate = useNavigate();
  • navigate('/home');

**3. Switch → Routes:**
Enter fullscreen mode Exit fullscreen mode


diff

  • } />

## Migration Approach

**Completed:**
- [x] Updated all route definitions (34 files)
- [x] Replaced useHistory with useNavigate (67 locations)
- [x] Updated all tests (45 test files)
- [x] Updated type definitions

**Tested:**
- [x] All navigation flows work
- [x] URL params work
- [x] Nested routes work
- [x] Redirects work
- [x] 404 handling works

## Risks

**Large change:** 67 files modified
**Mitigation:** 
- Incremental testing (tested each route individually)
- Feature flag: `ENABLE_REACT_ROUTER_V6` (can rollback)
- Deployed to staging for 2 days (no issues)

## Related

Blocks #1240 (React 18 upgrade)
Migration guide: https://reactrouter.com/docs/en/v6/upgrading/v5
Enter fullscreen mode Exit fullscreen mode

Templates for Different PR Types

Bug Fix Template

Title: [Fix: Brief description of bug]

## Problem
[What's broken? Error message? Impact?]

## Root Cause
[Why did this happen?]

## Solution
[What did you change?]

## Testing
- [ ] Reproduced bug before fix
- [ ] Verified fix resolves issue
- [ ] Tested edge cases
- [ ] Added regression test

## Risks
[Any concerns?]

Fixes #[issue]
Enter fullscreen mode Exit fullscreen mode

Feature Template

Title: [Add: Brief description of feature]

## Why This Feature?
[User need? Business value? Use case?]

## Implementation
[Technical approach]

## Testing
- [ ] Manual testing completed
- [ ] Unit tests added
- [ ] Integration tests added
- [ ] Edge cases covered

## Screenshots
[Before/after if applicable]

## Related
Fixes #[issue]
[Design link]
Enter fullscreen mode Exit fullscreen mode

Refactoring Template

Title: [Refactor: What you're refactoring]

## Why Refactor?
[Technical debt? Performance? Maintainability?]

## Changes
[What's different?]

## Migration Path
[How do other devs adapt?]

## Testing
- [ ] All existing tests pass
- [ ] No behavior changes
- [ ] Performance verified

Related to #[issue]
Enter fullscreen mode Exit fullscreen mode

The Self-Review Checklist

Before submitting your PR, review it yourself:

**Before submitting:**

Title:
- [ ] Descriptive and searchable
- [ ] Follows format: [Action] [What] [Where/Why]

Description:
- [ ] Problem is clear
- [ ] Solution makes sense
- [ ] Testing is thorough
- [ ] Risks are addressed
- [ ] Screenshots included (if UI change)
- [ ] Links to related issues/docs

Code:
- [ ] Removed console.logs
- [ ] Removed commented-out code
- [ ] Removed TODO comments (or created issues)
- [ ] No merge conflicts
- [ ] CI passing

Polish:
- [ ] Spell-checked description
- [ ] Code is formatted (Prettier)
- [ ] Linted (ESLint)
Enter fullscreen mode Exit fullscreen mode

Common Mistakes to Avoid

❌ Mistake 1: WIP PRs Without Context

Title: WIP

Description: work in progress, don't review yet
Enter fullscreen mode Exit fullscreen mode

Better: Still provide context even if WIP

Title: [WIP] Add pagination to user table

## Goal
Implementing server-side pagination for tables with 1000+ rows

## Status
- [x] Backend API changes
- [x] Frontend pagination logic
- [ ] UI polish
- [ ] Tests

**Not ready for full review yet**, but happy to get feedback on approach.

**Specifically:**
- Does the API design make sense? (see `UserController.ts`)
- Is the pagination hook reusable enough? (see `usePagination.ts`)
Enter fullscreen mode Exit fullscreen mode

❌ Mistake 2: Giant Unexplained Diffs

Title: Updates

47 files changed, 3,847 insertions(+), 2,103 deletions(-)

Description: Made some changes
Enter fullscreen mode Exit fullscreen mode

Better: Explain large changes

Title: Upgrade to TypeScript 5.0 and fix type errors

## Changes

**67 files changed** - most changes are type fixes:

**Breaking type changes:**
- `strict: true` in tsconfig (caught 45 real bugs)
- `noUncheckedIndexedAccess: true` (array access now properly typed)

**Categories of changes:**
1. Added null checks (23 files)
2. Fixed 'any' types (15 files)  
3. Updated library type imports (18 files)
4. Misc type narrowing (11 files)

**Key files:**
- `tsconfig.json` - stricter settings
- `src/types/` - updated type definitions
- `src/utils/` - added type guards

**No behavior changes** - only type safety improvements
Enter fullscreen mode Exit fullscreen mode

❌ Mistake 3: No Screenshots for UI Changes

Visual changes without visuals = reviewers have to check out your branch.

Always include:

  • Before/after screenshots
  • Different screen sizes (mobile, tablet, desktop)
  • Different states (loading, error, success)
  • Screen recording for interactions

❌ Mistake 4: Assuming Reviewers Have Context

You've been thinking about this problem for hours. Reviewers have 30 seconds.

Bad:

Fixed the race condition
Enter fullscreen mode Exit fullscreen mode

Good:

Fixed race condition in useEffect when userId changes rapidly

## Problem
When user navigates quickly between profiles (rapid userId changes), 
multiple API calls fire simultaneously. The responses arrive out of order, 
causing wrong user's data to display.

Example: Click User A → User B → User C rapidly
Response order: C → A → B
Result: User B's data shows (wrong!)
Enter fullscreen mode Exit fullscreen mode

Making Reviewers' Lives Easier

1. Add Comments to Complex Code

// In the PR description: ""
## Reviewer Notes

Line 45-67: Complex memoization logic
The triple-nested useMemo is intentional. We need to:
1. Memoize the filter function (inner)
2. Memoize the filtered data (middle)  
3. Memoize the sorted result (outer)

Tried simpler approaches but they caused performance issues with large datasets.
Enter fullscreen mode Exit fullscreen mode
// In the code:
// NOTE: This triple-nesting is intentional for performance.
// Each level prevents re-computation when only specific deps change.
// See PR description for alternatives considered.
const sorted = useMemo(() => {
  const filtered = useMemo(() => {
    const filterFn = useMemo(() => {
      // ...
Enter fullscreen mode Exit fullscreen mode

2. Explain Non-Obvious Decisions

## Design Decisions

**Why setTimeout instead of debounce?**
Debounce would delay the API call, but we want immediate feedback. setTimeout 
ensures button re-enables even if API call fails.

**Why not use a library for this?**
Evaluated `react-use` and `lodash`, but both added 20KB for a 5-line solution. 
Not worth the bundle size for this simple use case.
Enter fullscreen mode Exit fullscreen mode

3. Point Out Potential Concerns

## Reviewer Notes

**Line 123:** You might think this could cause a memory leak, but it's fine 
because we clean up in the useEffect return function (line 145).

**Performance:** This does add an extra render, but only in the error case, 
which is rare. Happy path performance is unchanged.
Enter fullscreen mode Exit fullscreen mode

The Ultimate PR Description

Here's a complete example that does everything right:

Title: Fix race condition in user profile when navigating rapidly

## Problem

**What's broken:**
When users navigate rapidly between profiles (clicking different users in quick 
succession), the wrong user's data displays.

**Why it happens:**
Multiple API calls fire simultaneously, but responses arrive out of order due 
to varying response times. The last response "wins" even if it's for an older 
navigation.

**Example scenario:**
1. User clicks Profile A (slow server, 2s response)
2. User clicks Profile B (fast server, 0.5s response)
3. Profile B loads (correct)
4. Profile A loads and overwrites Profile B (wrong!)

**Impact:**
- Affects power users who navigate quickly (~5% of users)
- Reported by 8 users in last 2 weeks
- Privacy concern: Users might see wrong person's data
- No data corruption (read-only), but poor UX and trust issue

**Root cause:**
useEffect doesn't cancel previous fetch when userId changes.

## Solution

Implemented AbortController to cancel in-flight requests:

1. Create AbortController in useEffect
2. Pass signal to fetch
3. Cancel previous request on cleanup
4. Ignore responses from cancelled requests

**Why AbortController:**
- Native browser API (no library needed)
- Proper way to cancel fetch requests
- Works across all modern browsers

**Alternatives considered:**
1. ❌ Track request IDs - complex, error-prone
2. ❌ Debounce navigation - poor UX, feels sluggish  
3. ✅ AbortController - clean, standard approach

**Code changes:**
- Modified `useUserProfile.ts` hook
- Added cleanup function to useEffect
- Added error handling for abort errors (expected, not logged)

## Testing

**Manual testing:**
- [x] Rapid navigation (10 users in 5 seconds) - correct user always shows
- [x] Slow network (throttled to 3G) - race condition fixed
- [x] Fast navigation then back button - works correctly
- [x] Navigation during loading - cancels and loads new user

**Automated tests:**
- [x] Unit test: AbortController cleanup
- [x] Unit test: Ignore aborted responses
- [x] Integration test: Rapid navigation scenario
- [x] All existing tests pass (87/87)

**Edge cases:**
- [x] Navigate away while loading - request cancelled
- [x] Navigate to same user - no unnecessary requests
- [x] Network error during navigation - error state shows correctly

**Browser testing:**
- [x] Chrome 120 ✓
- [x] Safari 17 ✓
- [x] Firefox 121 ✓
- [x] Mobile Safari iOS 17 ✓

## Performance Impact

**Before:** 
- 3 simultaneous requests for rapid navigation
- Wasted bandwidth
- Wrong data displayed

**After:**
- Only 1 active request at a time
- Previous requests cancelled
- Correct data always displayed

**Metrics:**
- No performance regression
- Actually reduces network traffic (fewer parallel requests)
- Memory usage unchanged

## Risks

**Low risk:**
- Small, focused change (1 file, 10 lines)
- Well-tested pattern (used in production by Facebook, Google)
- No database changes
- No API changes

**Browser support:**
- AbortController supported in all modern browsers
- IE11: Not supported, but we don't support IE11 anymore

**Rollback plan:**
- Feature flag: `USE_ABORT_CONTROLLER` (default: true)
- Can disable if issues arise
- No data migration needed

**Monitoring:**
- Added logging for abort errors (should be 0 in production)
- Dashboard: "Profile Load Success Rate" (should remain 99.8%)

## Screenshots

**Before:** Wrong user data shows after rapid navigation
![before](https://i.imgur.com/before.png)

**After:** Correct user always shows, old requests cancelled
![after](https://i.imgur.com/after.png)

**DevTools Network tab:**
Shows requests being cancelled (status: "cancelled")
![network](https://i.imgur.com/network.png)

**Demo video:** https://www.loom.com/share/abc123
Shows rapid navigation - correct user always displays

## Related

Fixes #1234
Related to #1200 (performance epic)
Similar fix: #1180 (same pattern for posts)

**Documentation updated:**
- Added AbortController pattern to wiki
- Updated `CONTRIBUTING.md` with example

## Reviewer Notes

**Focus areas:**
- `useUserProfile.ts` lines 23-45 - Main change
- `useUserProfile.test.tsx` - New test coverage

**Questions:**
1. Line 34: Should we retry on abort? I chose no, but open to discussion.
2. Line 42: Error logging - is this the right level (debug vs info)?

**Implementation details:**
- The cleanup function (line 45) runs on both unmount AND before next effect
- This ensures we cancel even if userId changes multiple times quickly
- AbortError is expected and not a real error, so we filter it in error logging

**Alternative approaches considered:**
See "Solution" section above. Happy to discuss if you prefer a different approach.
Enter fullscreen mode Exit fullscreen mode

Using AI to Generate PR Descriptions

AI tools like Claude Code, GitHub Copilot, and Claude can dramatically speed up PR description writing. But use them wisely—AI should assist, not replace your thinking.

The AI Workflow

Step 1: Generate the diff context

# Get your changes
git diff main...your-branch > changes.diff

# Or for staged changes
git diff --cached > changes.diff
Enter fullscreen mode Exit fullscreen mode

Step 2: Prompt the AI

Here's my go-to prompt for Claude:

I'm creating a PR. Here's my git diff:

[paste diff]

Please write a PR description following this template:

## Problem
[What bug/need does this address? Impact?]

## Solution  
[High-level approach, key decisions]

## Testing
[What testing was done?]

## Risks
[Any concerns or limitations?]

Generate a draft PR description. Focus on the "why" and "what", not 
line-by-line code explanation.
Enter fullscreen mode Exit fullscreen mode

Step 3: Review and refine

AI gives you a solid first draft. Now add:

  • Business context AI doesn't know
  • Testing you actually performed
  • Screenshots (AI can't generate these)
  • Links to related issues
  • Specific reviewer questions

What AI Is Great At

✅ Summarizing code changes

// You changed 15 files
// AI reads the diff and says:

"This PR refactors authentication to use OAuth2 instead of session cookies.
Changes include:
- Replaced SessionAuth with OAuth2Provider
- Updated login flow to redirect to auth server
- Added token refresh logic
- Migrated existing users to new auth system"
Enter fullscreen mode Exit fullscreen mode

✅ Identifying patterns

AI notices: "This change adds null checks in 8 files - looks like you're 
fixing a null reference issue throughout the codebase."
Enter fullscreen mode Exit fullscreen mode

✅ Suggesting test cases

AI suggests: "Based on the null checks added, you should test:
- User with null email
- User with empty string email
- User with whitespace-only email"
Enter fullscreen mode Exit fullscreen mode

✅ Writing first draft quickly
Instead of 10 minutes writing from scratch, AI gives you a 2-minute starting point.

What AI Needs Your Help With

❌ Business context
AI doesn't know:

  • How many users this affects
  • Why this matters to the business
  • Customer complaints that triggered this
  • Revenue impact

You add:

## Problem

[AI wrote: "Users with null emails can't access their profiles"]

Users with null emails (legacy data from 2019 migration) crash when clicking 
"Edit Profile". 

**Impact:** ← YOU ADD THIS
- Affects ~500 users (0.5% of active users)
- Support tickets: 12 in last week  
- Blocking users from updating notification preferences
Enter fullscreen mode Exit fullscreen mode

❌ Why you chose this approach
AI can see what you did, but not why you rejected alternatives.

You add:

## Solution

[AI wrote: "Added null check before processing email"]

Added null check before processing email

**Why this approach:** ← YOU ADD THIS
Considered three options:
1. Backfill null emails with defaults - rejected (creates invalid data)
2. Disable profile for null emails - rejected (poor UX)
3. Show "Add email" prompt - chosen (guides users to fix root cause)
Enter fullscreen mode Exit fullscreen mode

❌ Actual testing performed
AI can suggest tests, but doesn't know what you actually ran.

You replace AI's generic checklist with specifics:

## Testing

[AI wrote:]
- [ ] Tested happy path
- [ ] Tested edge cases
- [ ] Added unit tests

[You write:]
- [x] Tested with user ID 12345 (has null email) - shows prompt correctly
- [x] Tested with user ID 67890 (has email) - edit form works normally
- [x] Verified with @sarah-qa on staging environment
- [x] Added regression test in profile.test.tsx lines 145-167
Enter fullscreen mode Exit fullscreen mode

GitHub Copilot for PR Descriptions

If you use VS Code with Copilot, you can generate PR descriptions inline:

Method 1: Use Copilot Chat

@workspace Create a PR description for my current changes.
Include problem, solution, and testing sections.
Enter fullscreen mode Exit fullscreen mode

Method 2: In the GitHub PR UI

GitHub Copilot can analyze your diff automatically and suggest:

  • PR title
  • Description draft
  • Relevant reviewers

Pro tip: Copilot is good at titles, decent at summaries, but weak on context. Use it for the skeleton, then flesh it out.

Claude Code for PR Descriptions

Claude Code excels at analyzing codebases and providing context.

Example workflow:

# In your terminal with Claude Code
claude "Analyze my recent changes and write a PR description. 
Focus on why these changes matter and what problems they solve."
Enter fullscreen mode Exit fullscreen mode

Claude Code can:

  • Read your entire codebase for context
  • Understand related files you didn't change
  • Identify breaking changes
  • Suggest migration paths

Example output:

I analyzed your changes across 12 files. This appears to be migrating 
from Redux to Zustand for state management.

## Problem
The current Redux implementation has several issues:
- 200+ lines of boilerplate (actions, reducers, selectors)
- Causes unnecessary re-renders (I see 15+ useSelector calls that 
  trigger on any state change)
- Difficult to test (requires store wrapper in tests)

## Solution  
Migrated to Zustand for cleaner state management:
- Reduced from 200 lines to 45 lines
- Selective subscriptions prevent unnecessary re-renders
- Tests simplified (no Provider needed)

I noticed you kept the old Redux store in `store/legacy/` - is this for
gradual migration?
Enter fullscreen mode Exit fullscreen mode

Advanced: Iterative Refinement with AI

Don't settle for the first draft. Iterate:

Round 1: Generate basic description

Claude, here's my diff. Write a PR description.
Enter fullscreen mode Exit fullscreen mode

Round 2: Add context

Good start. Add that this affects 500 users and we've had 12 support 
tickets about it. Also mention we considered backfilling emails but 
rejected it because [reasons].
Enter fullscreen mode Exit fullscreen mode

Round 3: Focus on testing

Expand the testing section. I tested with users 12345 and 67890, 
verified the fix in staging, and added regression tests. Make it 
more specific.
Enter fullscreen mode Exit fullscreen mode

Round 4: Add reviewer notes

Add a "Reviewer Notes" section. I want reviewers to focus on the 
null check logic in ProfileForm.tsx and the test coverage in 
profile.test.tsx.
Enter fullscreen mode Exit fullscreen mode

The Hybrid Approach (Best Practice)

## My Workflow

1. **AI generates first draft** (2 minutes)
   - Paste diff into Claude
   - Get basic structure

2. **I add business context** (3 minutes)
   - Why this matters
   - Impact numbers
   - Customer feedback

3. **I add specifics** (3 minutes)  
   - Actual testing performed
   - Decisions made and why
   - Risks and concerns

4. **AI polishes** (1 minute)
   - "Claude, make this more concise"
   - "Reorganize for clarity"

**Total time:** 9 minutes
**Quality:** Better than 30 minutes writing alone
Enter fullscreen mode Exit fullscreen mode

Example: AI-Assisted PR Description

What I gave AI:

Here's my diff. Write a PR description.

[paste 15-file diff for race condition fix]
Enter fullscreen mode Exit fullscreen mode

What AI generated (2 minutes):

## Problem
The application experiences race conditions when rapidly navigating between 
user profiles, causing incorrect data to display.

## Solution
Implemented AbortController to cancel in-flight requests when the user 
navigates to a different profile.

## Testing
- Tested rapid navigation
- Added unit tests for cleanup
- Verified error handling
Enter fullscreen mode Exit fullscreen mode

What I added (5 minutes):

## Problem
The application experiences race conditions when rapidly navigating between 
user profiles, causing incorrect data to display.

**Impact:** ← ADDED
- Reported by 8 users in last 2 weeks
- Affects power users who navigate quickly (~5% of users)  
- Privacy concern: Users might see wrong person's data

**Example scenario:** ← ADDED
1. Click Profile A (slow response, 2s)
2. Click Profile B (fast response, 0.5s)
3. B loads correctly
4. A loads and overwrites B (wrong!)

## Solution
Implemented AbortController to cancel in-flight requests when the user 
navigates to a different profile.

**Why AbortController:** ← ADDED
- Native browser API (no library needed)
- Standard approach used by React Query, SWR
- Proper way to cancel fetch requests

**Alternatives considered:** ← ADDED
1. Request IDs - too complex
2. Debounce navigation - poor UX
3. AbortController - chosen for simplicity

## Testing
- Tested rapid navigation
- Added unit tests for cleanup
- Verified error handling

**Specifically:** ← ADDED
- [x] Navigated between 10 users in 5 seconds - correct user always shown
- [x] Tested on throttled 3G network
- [x] Verified with @sarah-qa in staging
- [x] Added test in useUserProfile.test.tsx lines 145-167

## Screenshots ← ADDED ENTIRE SECTION
**Before:** Wrong user data after rapid navigation
![before](https://i.imgur.com/before.png)

**After:** Correct user always shows
![after](https://i.imgur.com/after.png)

**Demo:** https://www.loom.com/share/abc123

Fixes #1234 ← ADDED
Enter fullscreen mode Exit fullscreen mode

Result: Professional PR description in 7 minutes instead of 15.

Prompts That Work

For initial draft:

Analyze this git diff and create a PR description with:
1. Problem section (why this change is needed)
2. Solution section (high-level approach)
3. Testing section (what should be tested)

Focus on user impact and business value, not implementation details.

[paste diff]
Enter fullscreen mode Exit fullscreen mode

For refactoring PRs:

This is a refactoring PR. Explain:
1. What problems exist with current code
2. What the refactored approach improves
3. Why this refactor is worth doing now
4. Any migration needed for other developers

[paste diff]
Enter fullscreen mode Exit fullscreen mode

For bug fixes:

This PR fixes a bug. Help me write a description that includes:
1. The exact error users experience
2. Why the bug happens (root cause)
3. How the fix solves it
4. What testing proves it's fixed

[paste diff]
Enter fullscreen mode Exit fullscreen mode

For polishing:

Here's my draft PR description. Make it:
- More concise (remove unnecessary words)
- More specific (replace vague statements)
- Better organized (improve flow)
- Professional (appropriate tone)

[paste your draft]
Enter fullscreen mode Exit fullscreen mode

When NOT to Trust AI

❌ Don't trust AI for:

  1. Exact numbers

    • AI might say "affects many users"
    • You say "affects 500 users (0.5%)"
  2. Testing claims

    • AI might say "thoroughly tested"
    • You say exactly what you tested
  3. Business context

    • AI doesn't know your customers
    • AI doesn't know your roadmap
    • AI doesn't know internal discussions
  4. Security implications

    • AI might miss that you're exposing data
    • AI might not flag authentication changes
    • Always review security-related descriptions carefully

✅ Do trust AI for:

  1. Code summaries - What changed across files
  2. Test suggestions - What you should test
  3. Structure - Organizing your thoughts
  4. First draft - Getting started quickly

The AI Safety Checklist

Before submitting an AI-generated PR description: ""

**Review these:**

- [ ] Business context is accurate (not AI assumptions)
- [ ] Numbers are real (not AI estimates)
- [ ] Testing reflects what you ACTUALLY did
- [ ] No hallucinated details (AI making things up)
- [ ] Security implications are addressed
- [ ] Links are correct (AI might invent URLs)
- [ ] Removed AI hedging language ("might", "could", "possibly")
- [ ] Tone is appropriate (not too formal or casual)
Enter fullscreen mode Exit fullscreen mode

Real Example: AI Mistakes to Fix

AI generated:

## Testing
The changes have been thoroughly tested across multiple scenarios and 
edge cases. All tests pass successfully.
Enter fullscreen mode Exit fullscreen mode

Problems:

  • Vague ("thoroughly tested")
  • No specifics
  • Sounds like AI boilerplate

You fix it:

## Testing

**Manual:**
- [x] Tested with user ID 12345 (null email) - prompt shows correctly
- [x] Tested with user ID 67890 (valid email) - form works normally
- [x] Rapid clicking "Add email" button - no issues

**Automated:**
- [x] Added unit test: `profile.test.tsx` lines 145-167
- [x] All existing tests pass (87/87)
- [x] Coverage increased from 78% to 82%
Enter fullscreen mode Exit fullscreen mode

Tools Comparison

Tool Best For Limitations
Claude Deep analysis, context understanding, refinement No direct Git integration
Claude Code Full codebase context, terminal integration Requires Claude subscription
GitHub Copilot In-IDE suggestions, PR UI integration Less context, shorter responses
ChatGPT General descriptions, formatting help No code context by default

My recommendation: Use Claude or Claude Code for complex PRs (refactors, large features). Use Copilot for quick bug fixes and small changes.

The Bottom Line on AI

AI is a 10x multiplier for PR descriptions:

  • 15 minutes → 5 minutes with better quality
  • Blank page problem solved (AI gives you a start)
  • Catches things you forgot (suggested tests, edge cases)

But AI is not a replacement:

  • You still need to add context
  • You still need to verify accuracy
  • You still need to add human judgment

Best practice:

AI writes the skeleton.
You add the soul.
Enter fullscreen mode Exit fullscreen mode

Use AI to:

  • ✅ Generate structure quickly
  • ✅ Summarize code changes
  • ✅ Suggest test cases
  • ✅ Polish your writing

Don't use AI to:

  • ❌ Replace your thinking
  • ❌ Make up business context
  • ❌ Claim testing you didn't do
  • ❌ Skip the review process

The workflow that works:

  1. Let AI generate first draft (2 min)
  2. Add your knowledge (5 min)
  3. Let AI polish (1 min)

Total: 8 minutes for a great PR description.

Conclusion

Good PR descriptions:

  • ✅ Save reviewers time
  • ✅ Get faster approvals
  • ✅ Reduce back-and-forth
  • ✅ Prevent bugs from slipping through
  • ✅ Create better documentation

Bad PR descriptions:

  • ❌ Waste reviewers' time
  • ❌ Sit for days
  • ❌ Require multiple review rounds
  • ❌ Let bugs slip through
  • ❌ Leave no documentation

The formula:

Problem (why) + Solution (what) + Testing (proof) = Fast approval
Enter fullscreen mode Exit fullscreen mode

My challenge to you:

Tomorrow, write a PR description using this template. Time yourself:

  • How long to write: ~10 minutes
  • How long to get approved: Probably 10x faster

That 10 minutes saves everyone hours.


What's your PR description template? Share your best practices!

Top comments (0)