GitHub Pull Request Reviews: Mechanics and Practice

A review on GitHub is a batch of comments plus a verdict, submitted together.

That structure is deliberate and worth working with rather than around. Comments accumulate as you read, and are published in one event when you submit. The alternative — commenting line by line as you go — sends a notification per comment and lets the author start responding to your first thought before they have seen your fifth.

Before you start

• Pull Requests Explained

The three verdicts

Verdict	Meaning	Effect on merging
Approve	This should merge	Counts toward required approvals
Request changes	This should not merge as it stands	Blocks merging where policy enforces it
Comment	Observations, no position	None

The distinction between comment and the other two is more consequential than it looks. A plain comment records no state: it does not block, does not approve, and does not satisfy any requirement. Leaving detailed feedback as a comment review on a repository with required approvals means the pull request is neither blocked nor progressed — it simply waits.

Request changes is the one people avoid, generally out of politeness. It is worth using when you mean it: it is the only signal that mechanically prevents a merge, and on repositories where it dismisses on new pushes, it is also the signal that guarantees you are asked to look again.

Authors cannot approve their own pull request

GitHub does not permit self-approval. On a solo repository this means required approvals cannot be satisfied at all — which is why single-maintainer projects either skip required reviews or configure a bypass. This surprises people setting up policy on their own repository for the first time.

Comment types

Line comments attach to a specific line in the diff. Most review feedback should be a line comment, because location is context.

Multi-line comments span a range — useful when the problem is a block rather than a line.

Review summary is the overall message attached to the verdict. Use it for the things that are not about one line: overall approach, missing tests, questions about scope.

Suggested changes are line comments containing a concrete replacement, which the author can commit with one click:

```suggestion
    timeout = min(base_delay * (2 ** attempt), MAX_BACKOFF)
```

Suggestions are the highest-value review tool available and are consistently under-used. For anything you could fix faster than you could describe — a typo, a naming inconsistency, an off-by-one — a suggestion removes an entire round trip. It also removes ambiguity: the author knows exactly what you meant, because you wrote it.

Conversations and resolution

Each line comment starts a thread. Threads can be marked resolved.

The convention worth adopting: the person who raised a thread resolves it, once satisfied. Authors resolving their own threads to clear the display is how genuine feedback gets lost — a resolved thread is collapsed, and collapsed feedback is unread feedback.

Where policy requires all conversations resolved before merging, resolution becomes part of mergeability rather than housekeeping, and this convention matters more.

Stale reviews and re-review

When a pull request is approved and then updated, the approval refers to code nobody has now read.

Repositories can be configured to dismiss stale approvals on new pushes, which discards approvals whenever the head branch changes. It is the safe setting and it is not free: a one-character typo fix invalidates three approvals, and on a large pull request that is expensive enough that people start batching fixes to avoid it.

Requesting re-review after substantial changes:

gh pr edit PULL_NUMBER --add-reviewer teammate-username

Tell the reviewer what changed

Re-requesting review with no comment asks someone to re-read the whole diff. A one-line summary — “addressed the backoff cap and added the missing test; the interface question is unresolved” — turns a full re-read into a targeted one. This is the single cheapest courtesy in code review.

Reviewing from the terminal

gh pr list --search "review-requested:@me"
gh pr diff PULL_NUMBER
gh pr view PULL_NUMBER --comments
gh pr review PULL_NUMBER --approve --body "Looks good — thanks for the extra test."
gh pr review PULL_NUMBER --request-changes --body "The retry loop can still spin unbounded; see comment."
gh pr review PULL_NUMBER --comment --body "One question about scope, no objection."

Checking out the branch is worth doing for anything non-trivial. Reading a diff tells you what changed; running the code tells you whether it works:

gh pr checkout PULL_NUMBER

What it doesChecks out the pull request's branch locally, so you can run and test it.

Why we run itSome categories of problem — behaviour under real input, performance, integration with the rest of the system — are invisible in a diff. This works for pull requests from forks too.

Expected resultBranch creation and checkout output.

What to actually look for

Reviews drift toward the things easiest to notice — formatting, naming — and away from the things that matter. A rough priority:

1. Does it do what it claims? Read the description, then check the code delivers it.
2. Is it correct at the edges? Empty input, concurrent access, failure paths, the retry that never terminates.
3. Is it safe? Injection, authorisation, secrets, unvalidated input from outside.
4. Will it be understandable in a year? Naming and structure, judged by a stranger’s standard.
5. Is it tested? Not coverage percentage — is the risky part exercised?
6. Style. Last, and ideally automated so no human spends attention on it.

If a linter can catch it, a human should not be reviewing it. Every formatting comment is attention not spent on correctness.

Reviewing large or generated diffs

Some pull requests contain thousands of changed lines. Two rules make them tractable.

Separate generated from written. Lock files, compiled assets and generated clients do not need line-by-line review; they need a check that they were regenerated by the right process. A .gitattributes marking paths as linguist-generated=true collapses them in the diff, which makes the human-written part visible.

Review by commit when the history is clean. If the author separated their work into meaningful commits, reviewing commit by commit is far easier than reading a combined diff — which is a practical argument for the history hygiene in Editing Commit History.

If a pull request is genuinely too large to review, saying so is the correct review. Approving something you did not read is worse than blocking it.

Review etiquette

The mechanics are easy; the practice is where review succeeds or fails.

Review promptly. A review that arrives in two hours is worth more than a better one that arrives in three days, because the author still has the context loaded.

Comment on the code. “This function is confusing” is about the code. “You always write confusing code” is not.

Distinguish blocking from optional. Prefix non-blocking thoughts — many teams use nit: — so the author knows what must change and what is a suggestion.

Ask rather than assert when unsure. “What happens if this is empty?” invites an answer. “This breaks on empty input” invites a defence, and is embarrassing when it does not.

Approve when it is good enough. Not perfect — good enough. Perfection-blocking is how review becomes a bottleneck and how people learn to route around it.

Say what is good. Rare, cheap, and it materially changes whether people want their code reviewed.

Security in review

Some things deserve specific attention because they are easy to miss and expensive to get wrong:

• New dependencies — who maintains it, and does it need to exist?
• Anything touching authentication, authorisation or session handling
• Input crossing a trust boundary, and whether it is validated
• Secrets: any literal that looks like a credential, and any new logging that might print one
• Changes to CI workflow files, which run with repository credentials
• Permission changes in workflows or Apps

For pull requests from forks, remember the CI restrictions exist precisely because the code is untrusted — and that reviewing a fork’s changes to a workflow file deserves particular care.

Reviewing efficiently

Review is a scarce resource, and a repeatable approach makes it go further.

Read the description first. Knowing what the author intended lets you evaluate whether the code achieves it. Reading the diff cold means reconstructing intent from implementation, which is slower and less reliable.

Take one pass for shape, one for detail. The first pass answers “is this the right approach?” — and if the answer is no, the detailed pass would have been wasted. Skimming the file list before reading anything is usually enough.

Deal with the biggest thing first. If the approach is wrong, say so before commenting on variable names. Twelve style comments followed by “actually, this whole thing should be somewhere else” is a poor experience.

Timebox it. If a review will take more than an hour, that is information: say so, and ask whether the change can be split. Approving something you did not fully read is worse than declining to review it.

Batch your comments. GitHub’s review model exists for this. Commenting line by line as you go sends a notification per comment and lets the author start responding to your first thought before seeing your fifth.

Reviewing a change in an unfamiliar area

You will be asked to review code you do not know well. That review is still worth doing, and it is worth being honest about its scope.

Things you can evaluate without domain knowledge:

• Does it do what the description says?
• Is the error handling plausible — are failures handled or swallowed?
• Are the tests testing behaviour rather than implementation?
• Is anything obviously unsafe: unvalidated input, a credential, a permission change?
• Is it understandable? If you cannot follow it, that is a finding, not a gap in you.

Things you cannot: whether the algorithm is right for the domain, whether the performance characteristics are acceptable, whether it matches conventions you have not seen.

Saying which you did is the useful part. “Approving on structure and tests; I have not evaluated the scheduling logic” is a genuinely helpful review, and far better than either a silent approval or declining.

When review goes wrong

Two failure modes, with opposite causes.

Review as a gate. Reviewers block on preference, request changes over style a linter should catch, and treat approval as an endorsement they will be judged on. Changes take days. People batch work to avoid the process, batches get larger, review gets worse.

The fix is cultural and specific: automate style entirely, distinguish blocking comments from suggestions with a prefix, and approve at “good enough” rather than “as I would have written it”.

Review as theatre. Approvals arrive in ninety seconds on four-hundred-line diffs. The requirement is satisfied and nothing is examined. This is usually a symptom rather than a cause — of pull requests too large to review, or reviewers with no time.

The fix is smaller pull requests, not stricter policy. Adding a second required approval to a rubber-stamping culture produces two rubber stamps.

Review your own pull request first

Open your own diff and read it as though someone else wrote it, before requesting review. You will find the debug statement, the commented-out block, the accidentally committed file, and at least one thing you would have been asked about. It costs two minutes and removes a whole round trip.

Reviews through the API

Everything here has an API equivalent, which is how review automation works:

gh api repos/OWNER/REPO/pulls/128/reviews \
  --jq '.[] | [.user.login, .state, .submitted_at] | @tsv'

gh api repos/OWNER/REPO/pulls/128/comments \
  --jq '.[] | [.user.login, .path, .line, (.body | .[0:60])] | @tsv'

The distinction between /reviews and /comments matters: reviews are the verdicts, comments are the line-level threads. A tool reporting “review activity” usually wants both, and they paginate independently.

Pull Request Automation covers submitting reviews programmatically, and the reasons to be cautious about automating approval specifically.

Common mistakes

Using a comment review when you mean approve or request changes. Records no state.

Authors resolving their own threads. Collapses feedback nobody read.

Reviewing formatting a linter could catch. Spends attention on the cheapest category.

Approving unread large diffs. The rubber stamp that makes required reviews meaningless.

Re-requesting review with no summary of what changed. Forces a full re-read.

Blocking on preference. If it is not wrong, it is a nit:.

Reviewing only the diff. Some problems are only visible when the code runs.

Exercise

Pair with someone, or use a second account.

1. Open a pull request containing a deliberate small bug and a deliberate style inconsistency.
2. As reviewer, leave one line comment on the bug and one suggested change for the style issue.
3. Submit as request changes, and confirm the merge is blocked.
4. As author, commit the suggestion directly from the interface and push a fix for the bug.
5. Confirm whether the earlier approval or block state changed after the push.
6. Re-review, approve, and merge.

Step 5 is the interesting one — whether the previous review survived the push tells you how this repository is configured for stale reviews.

Review as a teaching mechanism

For anyone new to a codebase, review is the fastest route to understanding it — and the review that teaches is written differently from the one that gates.

Explain the why, not just the what. “Use defer here” corrects the line. “Use defer here so the lock is released even if the block below panics” also prevents the next occurrence.

Link to the standard. A comment pointing at the existing convention, or at the lesson explaining it, is more useful than an assertion.

Ask about approach before detail. For someone learning the codebase, “have you seen how storage/ handles this? there is a helper” is worth more than five comments on the implementation they wrote without it.

Say what is good. Specifically. “The table-driven test here is exactly right” tells someone which of their instincts to keep, which no amount of correction does.

This costs a few extra sentences per review and compounds — every reviewer who explains reduces the number of times the same correction is needed.

Reviewing your own work

Before requesting review, read your own diff as though someone else wrote it.

gh pr diff
gh pr view --json files --jq '.files[] | "\(.additions)+ \(.deletions)- \(.path)"'

What this catches, reliably: debug statements, commented-out experiments, files you did not mean to include, unrelated whitespace changes, and at least one thing a reviewer would have asked about.

It also frequently catches something more useful — that the change is doing two things, and would be better as two pull requests.

Two minutes here removes a round trip that costs a day of latency. It is the single highest-return habit in this cluster and it requires no agreement from anyone else.

What you learned

• A review is a batch of comments plus one of three verdicts, submitted together.
• Only approve and request changes carry state; a comment review records none.
• Suggested changes remove a whole round trip and are consistently under-used.
• The person who raised a thread should resolve it.
• Stale-review dismissal trades safety against friction, and the trade is real.
• Review priority should run correctness, safety, clarity, tests — with style automated.

Review threads through the API

Review conversations have structure that the interface renders and the API exposes, which is worth knowing when you want to analyse or automate around them.

# Root comments — one per thread
gh api "repos/OWNER/REPO/pulls/128/comments?per_page=100" --paginate \
  --jq '.[] | select(.in_reply_to_id == null)
            | [.user.login, .path, (.line // .original_line), (.body | .[0:50])] | @tsv'

# Which threads are unresolved?
gh api graphql -f query='
  query($owner: String!, $repo: String!, $number: Int!) {
    repository(owner: $owner, name: $repo) {
      pullRequest(number: $number) {
        reviewThreads(first: 50) {
          nodes {
            isResolved
            isOutdated
            path
            comments(first: 1) { nodes { author { login } body } }
          }
        }
      }
    }
  }' -F owner=OWNER -F repo=REPO -F number=128 \
  --jq '.data.repository.pullRequest.reviewThreads.nodes[]
        | select(.isResolved | not)
        | {path, author: .comments.nodes[0].author.login}'

Resolution state is GraphQL-only — REST returns the comments but not whether their thread is resolved.

isOutdated is the field worth noticing: it marks a thread attached to a line that no longer exists after a push. Outdated threads are collapsed in the interface, which means an unresolved question can become invisible simply because the author pushed a change nearby. Checking for outdated-and-unresolved threads before merging catches feedback that was never actually addressed.

Related lessons

Required ReviewsNext: turning review from a habit into an enforced policy.

PR Best PracticesWhy review quality depends mostly on pull request size.

CODEOWNERSRouting reviews to the people responsible for the code.

gh prReviewing, checking out and commenting from the terminal.