What My Partner Taught Me About Code Reviews
I left a code review at eleven at night once that I have been thinking about for two years. It was three lines of feedback. They were technically correct. The author of the pull request did not push back. They updated the PR to address each of my comments, and the PR merged, and three months later they left the team. I do not think I caused them to leave. I think I was one of many who told them, in clean professional language, that their work was wrong without first telling them I had seen it.
I want to write about how I learned the difference between those two things, because I had been getting it wrong for years, and the person who showed me what right looks like has never reviewed a line of code.
I had been good at being right. I had built a reputation on being right. The way I was good was that I would read a pull request, identify what was wrong with it, and write the corrections in plain, polite language. I did not call people stupid. I did not embarrass anyone. I was, by the standard of code reviews most teams accept, a thoughtful reviewer. People said this about me. I said it about myself.
But I was leaving people. Not all at once. I was leaving them in the way you leave someone when you walk into the room with the answer already in your head and treat the conversation as a procedure for delivering the answer. The author of a pull request does not always need the answer. The author of a pull request needs, almost always, to be seen first, and then helped, and seen means, specifically, that the reviewer has understood what the author was reaching for before commenting on whether they got there.
I want to be careful about the word seen. It sounds soft. I do not mean it softly. I mean it in the engineering sense, which is also the human sense. To see what someone was reaching for is to construct, in your own head, a working model of the version of the change they were trying to make. With that model, you can comment on whether the implementation matched the intent. Without the model, you can only comment on whether the implementation matches your own intent, which is different, and the comment is not a review, it is a specification masquerading as feedback.
For most of my career, I had been doing the second thing and calling it the first.
The person who showed me the difference is my partner, and I want to be honest about how she showed me, because the honesty is the part that matters. She did not sit me down and explain it. She has a particular way of disagreeing with me, and I have been, for years, on the receiving end of the way, and the way is what taught me, slowly, what I had been missing.
When she disagrees with me, she does not say I disagree. She says, in some form, I want to make sure I understand what you are saying, because I think you might mean X, and I want to react to the right thing. And then she says back to me, in her own words, what I said. And I hear it back. And almost every time, before she has even gotten to her disagreement, I can feel, in my body, that what I said is not what I meant. The act of having it said back to me, by someone who is clearly trying to hear it accurately, surfaces something I would not have surfaced alone. I do not have to be told I am wrong. I notice I am wrong, by the cleanness of the mirror. The conversation moves, in the next second, to what I actually meant, which is often a different conversation than the one I had started, and we have it, and we are not on opposite sides of it, because she did not put us there, because she had not already decided what side of the conversation she was on before she found out what we were actually talking about.
I want to call this a technique. It is not a technique. A technique is something you can learn in an afternoon and deploy in a meeting and have it land badly because the deploying is the wrong layer. This is a posture. The posture is the prior decision that the thing she wants, in this conversation, is not to win it but to actually find out what I meant, and the difference between those two things, repeated over a thousand small moments, is the difference between a relationship that grows roots and a relationship that does not.
It is also, I am embarrassed to say, the exact missing skill in every code review I have ever left.
The unit of code review I had been writing was the correction. This should be X. Use Y here. We do not do this in this codebase. Each of these is, in some sense, useful information. None of them is review. The unit of code review that matters is the question, asked carefully enough to make sure you have understood what was being reached for. I think you were trying to make X explicit here. Is that right? What problem is this solving for you, specifically? I am not sure if this is meant to handle case Y or if Y is out of scope. Can you say more?
These look, on the surface, like soft questions. They look like you are pulling your punches. You are not pulling your punches. You are doing the thing you should have been doing the whole time, which is constructing, before you comment, an actual model of what the author was attempting. Once that model is in your head, your subsequent comments land on the right thing. They land on a real disagreement, which the author can engage with, instead of an imagined one, which the author can only update around.
The technical correctness of my old reviews was real. The corrections were not wrong. They just were not what review was for. Review was never about catching bugs. Bugs are caught more reliably by the type system, the test suite, and the slow accumulation of contact with the material. Review was about helping a specific human, who you will be working with for years, become the kind of engineer they are trying to become, in a way that does not break them in the process. The breaking, when it happens, is mostly invisible. People do not write into the team retro saying I was broken by code reviews. They just leave, eventually, and the people who remain tell themselves a story about why they left, and the story is almost never that the reviews were correct in their content and corrosive in their form.
I am not yet good at the new way. I want to be clear. I have years of muscle memory in the old way, and the new way takes longer, and the longer is uncomfortable when there is a queue of pull requests waiting and a deploy needs to go out by Friday. I revert to the old way often. I notice myself doing it, sometimes mid-comment, and I delete what I had written and start again with a question, and the starting again costs me twenty minutes I would have called productive in any other context, and I am trying to learn that the twenty minutes is the work, not the cost of the work.
I want to end with the small thing that surprised me about this, which is how the new way travels.
I started practicing it in code reviews because that was the room I had been getting it most wrong in. But the posture is the same posture in a meeting, and in an argument with a friend, and in the moment a stranger says something I instinctively want to correct. The posture is: before I respond to what I think you said, let me make sure I have heard what you actually said. The cost of the pause is about three seconds. The benefit, over a lifetime, is, I now think, almost everything.
My partner did not teach me this on purpose. She has just been doing it for the entire time I have known her, in conversations where the stakes were higher than any pull request, with patience that I have not yet earned and am trying to deserve. I am borrowing the posture, awkwardly, and bringing it back to the rooms where I work, because the rooms where I work are also rooms where humans live, and the humans are not, it turns out, that different from the one at home.
— Dallen Pyrah