by lo_zamoyski | 2025-09-25 15:55:57+00:00
It's a nice thought, but PRs, when more than performative, are often more about basic sanity checks than anything else.

There are two things that can be involved when a change is submitted: a design change and an implementation change.

Design changes that are significant enough to warrant review should be expressed and discussed in text, not code, in design documents of some kind. These tend to be quite general, but they establish agreement about general structure, which tends to be the most difficult to change later on, as it circumscribes and guides the implementation that follows. Writing is also a way of working out ideas. The very act of having to explain something clearly to someone else forces you to confront your own ignorance and the consequences of your proposal. Besides, it's the job of the person proposing the design to work out those consequences so that others can verify whether they're true. (1)

Reviewing implementation changes with an understanding of design allows the reviewer to understand how changes relate to the whole as well as the aim. This is an insider's perspective. (2)

Reviewing implementation changes without a good understanding of the context will be limited to general technical remarks, but can descend into excess attention given to style or inconsequential matters of taste. This is the outsider's view. (3)

The question, I think, that looms in the background is whether familiarizing yourself with the context sufficiently well so that you can judge the PR submission is reasonable for a PR. In many cases, it isn't. It's too time consuming and the context is too big. If we had infinite time, this would be great: having to explain to an outsider what you've done forces you to give a much more thorough account, if your goal is to achieve thorough understanding. It also exposes your thinking to someone who doesn't necessarily share your assumptions. But this can be a Herculean task for anything sufficiently complex. So the criticality of the change must be weighed against the effort needed to explain or learn. Are you making a change to something with high tolerance for error, or a small margin of error?

Two pieces of advice...

Since it is unrealistic to expect an exhaustive verification all the time, focusing more on tests will be more fruitful. You still need context to judge whether they're exhaustive or test the right things, but it's the one place where correctness criteria are expressed in code, apart from type signatures, in a clear enough manner that expectations can be judged. If they aren't clear, you should ask. It's a good locus for discussion.

The second: code doesn't include the rationale or "why" for what it is. It just is. Context goes a long way to help infer reason for the change. This means we should use comments, either in the code or the PR submission itself, to explain changes. If something isn't sufficiently clear, ask.

But the key is prudential judgement. You have to determine how to limit your verification efforts and when to begin accepting on trust for practical reasons.

And do away with your pride. It's only difficult to ask questions if you suffer from pride, and pride is a sure sign of mediocrity. You're also lying through omission.