Phase 3, week 22retrospective

Upstream PR review

Opening the PR is the start of review: each comment needs a code or rationale response, and the thread is the record.


The pull request is the opening move

Opening it starts the review, and the review is where a correct patch becomes an acceptable one. Each comment gets one of two answers: a code change, or a rationale with evidence. Evidence means a specification line, a benchmark, or a test that would fail otherwise. “It works on my machine” is not a rationale.

Answer in the thread

Push the change, then reply naming the commit that addressed the comment, and resolve the thread after the reply rather than before. A reviewer who has to re-read the whole diff to find out what you changed will review more slowly, and the thread is what a future reader sees. That future reader may be on a hiring loop.

When the design is contested

If the reviewer wants a different structure, that is a new commit series rather than a debate. Ask one question that resolves it: which case does the alternative handle that this one does not. If the answer is a case you missed, the reviewer is right and the patch is better for having asked.

Latency

Review is asynchronous and the queue is long. A short ping in the thread after a week is normal; a direct message is not. Rebase when the project asks, and if you force-push over commits a reviewer has commented on, say so in the thread. The diff may be tidier and the conversation will look confused.

Things that break

Replying “done” to every comment, so the reviewer must diff the branch to learn what changed. Arguing about style for a week when the style guide already answered. Silence after a push, which leaves the thread waiting on you rather than the other way round. And a description that still describes the first version of the patch.

Sources

A version records the documentation the claim was checked against, not a house pin. Confirm flags with --help on the release you installed.

Check yourself

What are the two acceptable responses to a review comment?

A reviewer asks for a different design. What is the move?

Why reply in the thread after pushing a fix?

0 / 3 correct

← back to the map