contributor-experience-review

Files (4)

• SKILL.md
• DETECTION-PLAYBOOK.md · references/
• QUALITY-FILTER.md · references/
• REVIEW-VOICE.md · references/

SKILL.md (first lines)

# Contributor Experience Review

A sequencing lens on top of [`pr-review`](https://github.com/priyank766/OpenSource-SKILL/blob/main/skills/pr-review/SKILL.md), for PRs from people who
have not contributed to this repo before.

**Read first:** [`DETECTION-PLAYBOOK.md`](references/DETECTION-PLAYBOOK.md) ·
[`QUALITY-FILTER.md`](references/QUALITY-FILTER.md) ·
[`REVIEW-VOICE.md`](references/REVIEW-VOICE.md)

> **Where the time goes.** Finding the flaw is the review. The playbook above is the
> general flaw-hunting procedure; this file is the domain-specific lens on top of it.
> Scoring and writing the comment are the last ten minutes, not the work.

**Severity floor: 8.5** — higher than the general floor of 8.0, not lower. A regular
contributor reads a 7.5/10 nit as a nit and ignores it. A newcomer cannot tell a blocking
issue from a preference, so every comment you post reads as a condition of merge, and a
review of twelve comments reads as a rejection. The floor goes up because the cost of noise
goes up. Cut harder, not softer.

---

## Governing rules

**The bar is the same.** Correctness, tests, scope, and API compatibility are judged
exactly as they would be for a core maintainer. Merging a weaker patch because the author
is new ships a bug to users, leaves the maintainer holding it, and teaches the contributor
that this project does not mean what it says. Only the *sequencing* changes.

**One round, not five.** A newcomer PR that takes four review round-trips usually loses the
contributor somewhere around trip three. Read the entire PR before writing anything.
Resolve every issue you intend to raise. Deliver it as one batch and say so in plain words:
"this is everything I found — nothing else is coming."

**Never leave them guessing what happens next.** Every review ends with one unambiguous
next action and a named owner: they push a fix, you push a fix, you merge on green, or you
close. A review that ends with observations and no instruction is an incomplete review.

**Unstated context is the actual defect.** Most newcomer PR problems are the project having
failed to write something down, not the person having failed to read it. If three people in
a row miss the changelog fragment, the changelog fragment is undocumented. Treat a recurring
newcomer mistake as a bug in `CONTRIBUTING.md` and fix it there.

---

## Why this exists

Projects lose contributors at a predictable point: the second round-trip on their first PR.
The usual cause is not hostility. It is a review that was technically fine and
operationally shapeless — nits mixed with blockers, no ordering, no statement of what
merging requires, delivered in three installments over five days.

The fix is not warmth. It is compression.

### What each role gets out of it

| Role | Time this gives back |
| :--- | :--- |
| **Maintainer** | Your real cost is round-trips, not comments. Each one is a context reload days later. Front-loading turns a five-touch PR into a two-touch PR, and triage-first stops you line-reviewing code that was never going to merge. |
| **Contributor** | One list, ordered, with the blocking items marked. You know what merging requires before you start, instead of discovering it one comment at a time. |
| **Project owner** | Retention. A contributor who lands a first PR cleanly often lands a fifth, and eventually reviews someone else's — the only durable fix for maintainer load. Each unnecessary round-trip is a measurable drop in that. |

---

## Procedure

```mermaid
flowchart TD
    A[New contributor PR] --> B[1. Establish authorship context]
    B --> C[2. Triage the PR type]
    C -->|"spam or metric farming"| D[Close per policy, no debate]
    C -->|"out of scope or needs design first"| E[Stop. Say so now, do not line-review]
    C -->|"viable contribution"| F[3. Full read, collect raw findings]
    F --> G[4. Sort: blocking / worth-fixing / ignore]
    G --> H[5. Score at floor 8.5, drop the rest]
    H --> I[6. Supply the missing project context]
    I --> J[7. Post as ONE batch with a next action]
    J --> K{Recurring newcomer mistake?}
    K -->|yes| L[Open a CONTRIBUTING.md PR]
    K -->|no| M[Done]
```

### 1. Establish authorship context

```bash
gh pr view <PR> --json author,createdAt,isDraft,files
gh api /repos/<OWNER>/<REPO>/pulls/<PR> --jq .author_association   # not a gh --json field
gh search prs --author <USER> --repo <OWNER>/<REPO> --state all
gh search prs --author <USER> --state all --limit 30      # pattern across all projects
gh pr view <PR> --json body --jq .body                    # does it link an issue?
gh issue view <ISSUE> --json assignees,state,labels        # was it assigned to someone else?
```

An `author_association` of `FIRST_TIME_CONTRIBUTOR` or `NONE` is the trigger. It is only
available from the REST API — `gh pr view --json` does not expose it. A cross-project
search returning thirty single-line PRs opened in one hour is the strongest slop signal
available. An unassigned linked issue is fine; an issue assigned to someone else three weeks
ago is a conversation to have before reviewing a line.

### 2. Triage first: what kind of PR is this?

Do this before any line-level reading. The correct response differs completely by category,
and reviewing code in the wrong category is pure waste.

**(a) Genuine newcomer contribution, technically viable.** Signals: linked issue, a PR
description in their own words, tests touched, questions asked in the thread, scope matches
the issue. *Handling:* proceed to the batched review below. This is the main path.

**(b) Viable, but out of scope, duplicating existing work, or against the roadmap.**
Signals: reimplements something already in `main` or in an open PR, adds a dependency the
project has previously rejected, changes a public interface the roadmap is about to
replace. *Handling:* **stop.** Say so in the first response, link the existing PR or the
decision, and do not line-review. Reviewing code that will not merge in this form spends
your time and theirs and makes the eventual "no" look arbitrary.

**(c) Low-effort or AI-generated with no understanding behind it.** Signals: sweeping
rewrites across unrelated files, invented APIs that do not exist in the codebase, comments
restating each line, a PR description that summarizes the diff instead of the intent,
tests that assert nothing, no reply or an equally generated reply when asked a specific
design question. *Handling:* ask one specific question only they could answer — "what

From priyank766/OpenSource-SKILL, Apache-2.0.

More from priyank766/OpenSource-SKILL