Writing PRs Maintainers Actually Merge

A merged PR is a tiny artifact with huge career value. An unmerged PR is the same code, wasted. The difference is rarely the code itself, it's how you present it, scope it, and engage with review.

The structure of a good PR description

## What this PR does

One sentence summary. Then 1-3 bullets if needed.

## Why

Link to the issue this addresses (`Closes #1234`) and a sentence on the
real-world problem.

## Approach

Brief, 1-3 paragraphs. Why this approach over alternatives.

## Tests

What's added/modified. How to verify.

## Open questions / notes

Anything reviewers should know. "I wasn't sure about X, happy to change."

Checklist:
- [x] Tests added/modified
- [x] CHANGELOG.md updated
- [x] Docs updated if behaviour changed
- [x] Linted / formatted per CONTRIBUTING.md

Five sections. Each <100 words. Reviewers should understand the intent in 60 seconds and the technical detail in 5 minutes.

The first message you send sets the tone of the entire review.

Match the project, not your preferences

Whatever conventions the project uses, match them.

• Their formatter? Run it.
• Their linter config? Pass it.
• Their commit message style (fix(parser): handle empty title)? Use it.
• Their test style (pytest fixtures vs unittest classes)? Mirror it.
• Their import order? Same.

This is unglamorous and the single biggest source of PR friction. Maintainers tire of saying "please format with black" on every contribution. Read CONTRIBUTING.md once, configure your editor, never have to think about it again.

Scope discipline

A PR should change one thing. The 50-line PR with one clear purpose merges. The 500-line PR mixing a bug fix, a refactor, and three unrelated style cleanups stalls forever.

If you find yourself wanting to fix three things while you're "in there":

• Fix the one the PR is about.
• File issues for the others (or note them in the PR description as "follow-ups").
• Submit separate PRs.

Three small merged PRs > one big rejected PR.

Tests

Almost all merged PRs to mature projects include tests. The pattern:

1. Write a failing test that demonstrates the bug or new feature.
2. Make the fix.
3. Verify the test now passes.
4. Run the rest of the suite to confirm no regressions.

For docs/typo PRs, tests aren't needed. For anything else, expect to add tests. Maintainers are often more rigorous about tests than core logic, they don't want to maintain code without a safety net.

Look at how existing tests in the project are structured. Mirror their style:

# Look at: tests/test_existing_thing.py
# Then write: tests/test_your_new_thing.py, same shape

Commit history

Some projects squash all commits at merge. Others require a clean commit history. Read the project's policy.

If they require clean history, before opening the PR:

# Make sure your branch is rebased on latest main
git fetch origin
git rebase origin/main

# Squash WIP commits into logical units (NOT into one giant commit)
git rebase main
# Mark some commits as `squash` or `fixup` interactively

Conventional commits (feat:, fix:, docs:, chore:) are widely adopted; use them unless the project uses something different.

Asking for review without being annoying

When you open the PR, maintainers may not see it for days. After ~1 week:

Friendly bump on this PR, happy to address any review when you have a moment. Let me know if I should change anything.

After ~2 weeks of no response, polite ping in a public discussion or Discord. Never aggressive.

Some projects have one part-time maintainer juggling a hundred PRs. Patience is the right baseline.

Responding to review comments

Three categories of comment:

1. Style/nit comments, just apply them. Don't argue formatting or "I prefer it this way." Match the project's preference.

2. Code-quality suggestions, apply them unless you have a substantive reason. Even then, explain calmly and accept the maintainer's call if they push back. Their codebase, their decision.

3. Design comments ("this approach won't work because X"), engage thoughtfully. Ask clarifying questions. Propose an alternative. Be open to "let's close this PR and rethink."

Never:

• Argue defensively in long comment threads.
• Resubmit the same PR ignoring feedback.
• Make passive-aggressive comments about "delays."

The OSS world is small. A maintainer who liked your PR may bring you in next time. One who didn't will avoid you indefinitely.

The "good enough" trap

Some contributors over-engineer first PRs, adding configuration options, abstract base classes, plugin hooks. Maintainers reject these.

Make the PR do exactly what's needed, no more. Generality earns its way in over multiple PRs as the use case proves itself. Premature flexibility is the most common reason simple PRs balloon into rejected proposals.

Concrete example: a great PR

Title: Fix RetryMiddleware to honour Retry-After header

Description:

What this PR does When a response has a Retry-After header, RetryMiddleware now uses that value to determine the next retry delay instead of always falling back to DOWNLOAD_DELAY.

Why Closes #5432. Real sites (Cloudflare, Twitter API, etc.) return Retry-After during rate-limit periods. Ignoring it leads to repeated bans.

Approach Added an _get_retry_delay() method to RetryMiddleware that parses Retry-After (both seconds-format and HTTP-date format). Falls back to existing exponential backoff if header absent.

Tests Added test_retry_after_seconds, test_retry_after_http_date, test_retry_after_missing in tests/test_downloadermiddleware_retry.py. All passing locally.

Notes I considered making this opt-in via a setting; opted against to match HTTP RFC behaviour. Happy to gate behind a setting if you'd prefer.

+52 / -3 lines, 3 commits, all tests pass.

This kind of PR merges quickly. Specific, well-tested, scope-disciplined, polite, with an explicit "happy to change" note.

What to try

Take an existing PR you've sketched out. Run it through this checklist:

1. Description has the 5 sections.
2. Scope is one thing.
3. Tests cover the change.
4. Style matches the project.
5. Commit history is clean.
6. You'd merge it if you were the maintainer reading it cold.

If any answer is no, fix it before submitting. The PR's success rate on first review tracks closely with how clean it is on submit.