How to not sound passive aggressive in code reviews?

Question

As a young guy in my first programming job, I can't figure out how to avoid sounding passive aggressive when providing code review comments or when responding to such comments in my own code. It invariably ends up sounding like "X does Y, not Z. It's not needed here." in response to something like "don't we need to call function X here?".

So how do you properly respond to general review comments like "I think this is missing here" or "I think this is not needed here"? And how do you correct a coworker's misunderstanding without sounding snappy?

I'm asking because a coworker said "I think this comment doesn't accurately describe the code because X actually does Y". I was going to respond with "X actually doesn't do Y. Z is the one that actually does Y". However, it just sounds slightly snarky. I for sure would be a little annoyed if someone responded to me like that.

Answer 1

He misunderstood your code.

Ensuring the team understands your code correctly was in order.

Your example states simple facts and would have been just fine as a reply.

The best answer would have been

"Actually Z does Y"

Concise, objective and precise without any personal context.

In professional conversations you stick to objective facts and established procedures.

You leave out personal comments or remarks, pointing fingers or attacking personally.

If someone made an obvious mistake that you're certain of or they misunderstood your own code, you're not only allowed but also obliged to correct them in a respectful manner, avoiding embarrassing or reprimanding them with other people present.

Stating objective facts is not passive aggression and anyone offended by them is unprofessional.

Answer 2

It depends on the environment and on the relationship you have with the colleague whose code you are reviewing (or that is reviewing your code). In some places (countries, teams, workplaces, etc...) being direct, even in code reviews, is not acceptable and considered offensive; in others being too indirect is considered silly. You need to adjust your style to the environment and the people you work with.

For instance, I worked in environments where

Replace X with Y, X doesn't do {whatever}.

was acceptable, and places were

This looks very good to me, but I believe you should use Y here, because X would....

sounded too direct.

I currently work with a colleague who is extremely proud of his work, and that tends to feel diminished even by constructive criticism. I work around this by reviewing his code with him sitting next to me and asking him to describe the code; it becomes a sort of delayed pair-programming session, where we discuss improvements and work them out together. During the process we add notes to the PR, so that he can go back to it later on. We go through a similar process if he reviews a PR of mine and I don't agree with one of his comments.

So, if you are in doubt, you should talk to your colleague, clarify in person and then reply to his comment. If the colleague is remote, you can achieve a similar result with a quick call or a few IMs. If you agreed on the content of your reply, even something like

As discussed, this is not correct, I'm resolving this conversation (To use a GitHub terminology)

won't sound snarky, because of the context.

Answer 3

The best way to correct anyone in a situation is by doing so in a manner that allows them to save face, especially when doing reviews like this.

The best way to do it is to do so indirectly.

Hmmmm... Let me just double check... Ah! Here we are. Z does Y. I'm sorry if I didn't comment that clearly enough.

Or something like that. You're right to feel that "X actually doesn't do Y. Z is the one that actually does Y". sounds snarky.

Another way you could phrase that is by gently nudging the person to walk through it again.

Let's check again.... Can you show me what's wrong?

and let the person find the mistake himself.

It's better to let a person save face than to be right.

Answer 4

I would like to add an approach I have found useful - correcting the reviewer's remark concisely, as answered above, but also offering to add a comment, change the documentation or rename the function to make its intent clearer ( if possible ).

Perhaps the reviewer rushed a bit and thus misunderstood your code. But the best code can be read and understood without much thinking, and this code apparently wasn't understood that easily.

Of course, not everything can be made completely simple. But a review comment like that indicates there may be room for improvement on readability here.

Keep in mind you or one of your coworkers may have to understand this code in a situation where there isn't much time or your mind is on other things. Such comments can then prevent misunderstandings and possible mistakes in the future - and just not having to think it through is often a boon in itself.

I've often found myself grateful to encounter a little comment explaining a function call, even in my own code. Things that are obvious when you're deep in the subject matter become less obvious over time, and a couple of words of explanation can save you a lot of frustration later ;)

This also serves to show respect to your coworker, in that it says "Perhaps I didn't make it clear enough", not "You misunderstood this, I'm right".

Answer 5

You didn't mention your location, and what you are asking is very location dependent. In the UK, you just state the facts. When you're sure, you say "X does Y, as it is supposed to." When you're not quite sure, you say "I think X does Y, as it is supposed to". There's no "saving face". I'm capable of doing some good work, but I'm also capable of doing outright stupid things, and I also know you are capable of doing outright stupid things, so if you catch me doing something stupid I'll just fix it.

I know there are countries where it doesn't work that way, where it is considered extremely rude to criticise a co-worker (to the point where airplanes have crashed because the co-pilot was too polite to point out the pilot's fatal mistake). I'd love to see a reply from someone working in India, for example.

Answer 6

First off, you can't worry too much about how someone else with interpret your comments or infer something from your supposed 'tone'.

I find often that people who accuse someone of 'passive-aggressive' behaviors do so from a defensive position after simple statements of fact cast them in a negative frame, even if the error is very easily recoverable. They just don't want to admit the mistake or oversight. For example:

Statement: The functional guidelines say Z does Y.

Reaction: OMG, why are you accusing me of not reading the spec? The docs aren't very well organized and the part about Z isn't very clear You need to chill out!

In a technical environment, simply stating facts is likely to have by far the broadest appeal and acceptance. If someone gets overly defensive, they have interpersonal issues and you just need to remember that for next time.

I almost universally find that people are reasonable and perfectly fine either accepting the situation or making corrections, even if they're at fault, when the error/oversight/bug is the issue, not the person.

Why doesn't Z do X? That's a really big deal!

Page 4 of Specification 2 say Z does Y, so that's what it does.

Oh, I see. Guess it must have change.

No problem. If Z does need to do X, just update the spec and we'll make it happen.