These days I make a living producing content (or, increasingly, running a content operation). As a result, social media and blogs have become more of a broadcast operation for me than a source of information. Still, I had occasion recently to meander out of my bubble and observe a debate about code review etiquette.
The Code Review Etiquette Conundrum
Here are the two sides of that debate, paraphrased in my best attempt to inject no subjective bias.
- Code reviews can tend toward depressing and demoralizing for the reviewee, so make sure to include some kind of positive feedback.
- Manufacturing compliments is patronizing, so just stick to the facts and action items.
For the remainder of the post, I'll use the relatively value-neutral framing of "subtlety vs. directness" to refer to these relative positions, with the idea that a continuum of sorts exists between them.
This incarnation was just the most recent. I've seen this debate a lot over the years, and I've certainly spoken about code reviews before:
- Getting an annoying reviewer off your back.
- Automate it as much as possible to minimize the subjective human element.
- How code review culture becomes toxic.
So, as you can probably infer, I would be more likely to come down on the first point in the debate: be polite. That's not only my natural preference (politeness or just not doing them), but the product of my personal background and culture. That's a strong statement, so let's but a brief pin it and then unpack it in detail.
We Assume the Subtlety vs Directness Question is a Conscious Choice of Personal Style
Reading the premise of this post, you probably start forming inner monologue immediately. "Would it really kill you to just be nice for once?" Or perhaps, "why would we waste a bunch of time beating around the bush with insincere games?"
That monologue then turns into the right way of doing things. "Being nasty and constantly negative causes corporate turnover and makes our industry toxic, so we should stop." Or, "all of these games waste time and dilute the review process, hurting our production code." So think pieces on the subject become like this one about "spare me the compliment sandwich." You naturally think that your style is the right one and then seek to argue that everyone else should adopt your correct approach.
Code Review Rendered Trivial: The Ethnic Theory of Plane Crashes
This looks a lot like the way we reason and argue about coding style. But subtlety vs. directness is not that at all. It's heavily cultural.
Years ago, I listened to an audio book called Outliers, by Malcom Gladwell. It's a fascinating book, but what's relevant here is its seventh chapter (summarized in detail here). And now, anytime I hear a debate like this about code review, I think of Gladwell's book and about Colombian pilots.
The Colombian pilots landed intensely in the subtlety camp. The air traffic controllers at JFK airport in New York were decidedly direct. And a completely avoidable tragedy ensued because of the impedance mismatch, so to speak, between these interaction models.
The Colombian pilots told the air traffic controllers that they were running low on fuel, and that they needed to land. But they didn't say (or yell) that they had an emergency on their hands, when, in fact, they did. So the air traffic controllers reasoned that there was, in fact, no emergency and continued to put them off from landing. The plane ran out of fuel and crashed.
If this approach can't find easy resolution in a life or death situation, what hope do we have around a Github pull request or an IDE projected on the conference room flat screen?
Different Cultures Land in Really, Really Different Places on the Subtlety vs. Directness Spectrum
As I thought about the code review bluntness debate and the beginnings of this post, I started googling idly. I seem to recall (though I don't have the book in front of me) that Gladwell talked a lot about different countries appearing in different places on this continuum. And, sure enough, anecdotal Googling seems to suggest that.
Here's some fun reading, for those interested:
- A Quora question on why Israelis are so blunt. People responding (presumably Israelis) defend this as sort of the right way to do things. "Why would you beat around the bush?"
- An anecdote about culture clash of sorts between relatively blunt Americans and a more indirect aspect of Mexican culture. Instead of asking someone to smoke, the "proper" thing would be to say that smoke "hurts you" and trust that the other person takes this social cue.
- Here's an article about why Germans are so blunt. It talks in some detail about how German is a much more literal/precise and less ambiguous language in terms of expressing ideas.
- This article explains why American culture (middle of the road-ish on bluntness) shocks Japanese nationals, who hail from a very subtle culture. Japanese, apparently, wouldn't want to be rude by expressing hunger to a host, whereas Americans assume that, if they were hungry, they would simply say so.
- This article talks about understanding Indian corporate culture and that not all answers of yes actually mean yes. Answering yes without hesitation means yes. Hesitating, then saying yes indicates reluctance. This is because telling someone "no" (particularly a superior) indicates disrespect.
Heterogeneous Subtlety-Directness Environments Are Realpolitik Landmines
I list all of this to call attention to the fact that this is not some kind of decision you make, like tabs or spaces. You grow up steeped in your own culture, which heavily informs whether you communicate with subtly or directness. And encountering wildly different cultural approaches creates culture shock.
The subtle person might think the direct one boorish. Interaction modes that seem normal to the direct person strike the subtle one as on par with them asking "can I watch you go the bathroom?" The resultant thought is, "what is wrong with you, you barbarian?!" Likewise, a direct person who asks, "do you mind if I eat while we go over this" might, quite reasonably take an assent response literally. So when the other party to the meeting goes around telling people that he's an uncultured troglodyte, he'll likely think "what a snake."
This isn't the result of a right way or a wrong way, nor is it the result of people being good or bad. It's the result of the fact that different cultures have subconscious interaction dances and rules. And when you find yourself in unfamiliar territory, you will naturally feel frustrated and that everyone around you is insane.
Code Review Etiquette Has Everything to Do with the Subtlety-Directness Continuum
Very early on, I mentioned my own cultural experience informing my code review opinions. I am a conflict avoider by nature, having once described myself as a lumpenappeaser. I am also an introvert. And, while perhaps some of this is genetics, I think that a lot of it is cultural upbringing.
In my house as a kid, you didn't argue with other family members in front of company. "If you don't have anything nice to say, don't say anything" definitely prevailed as guiding wisdom across the board. You don't offer people unsolicited opinions, especially ones that are directly critical. Mine was an upbringing of always striving to be polite, to avoid tension and arguments, and to keep life equanimious. When I would go over to friends' houses and observe yelling matches among family members, I would watch with cringing astonishment.
This certainly informed my approach to and preference with code reviews.
My Own Code Review Approach as an Example of Moderate-Subtlety
If you don't ask me my opinion, I won't offer it. If you do send things to me for code review, I'll phrase things very specifically. That is, I won't give you orders or express my opinions as fact. "That's not how I would do it" instead of "change that, it's wrong." I prefer leading people to possible mistakes by asking questions rather than poking holes. And I'll definitely sprinkle in compliments as much as possible to take the edge off of criticism. This is not because I want the same done to me (I don't care) but because it feels wrong to me to start slugging away with untempered negativity.
But I also recognize that this is just my preference.
People have in the past asked me to be more direct or give them "brutal honesty." I try, but that's just not me. I'd honestly rather leave non-critical warts in a codebase than risk relationship damage with sensitive people or than feel like I'm being boorish. We can always fix the code later.
What Your Behavior in Heterogeneous Subtlety-Directness Team Code Reviews Means for Your Career
If you come from a culture even subtler than mine, I'll probably find myself occasionally exasperated. If you come at me from a place of overwhelming bluntness, I will probably start to regard you as an enemy and an obstacle that I'll deal with by, say, transferring away or writing a popular post series and book about you (assuming that you were as marginally competent as you were blunt).
And I'm not alone in this. You'll have these impulses too, as will everyone around you with whatever subtlety-directness background they have.
So the first thing that I'd suggest you do, as a human, is understand this. Yeah, others have different proclivities about this than you do. And yeah, those proclivities will feel weird to the point of wrongness. But understand this and try to come to an accord. Be blunt in telling your blunt colleagues that their bluntness makes you uncomfortable. Be subtle in your feedback for your subtle colleagues, even as you encourage to be blunt with you and ask for their patience and non-offense.
But setting that aside, there's another element here at play as well -- the realpolitik element.
Navigating these cultural differences and preferences won't just help a mundane IC activity like code reviews go well -- it'll have important impact on your career. You demand that everyone conform to your cultural comfort at your own peril.
Code Review is Power-Theater
Readers of my book and examinations of pyramid-shaped corporations were probably expecting me to round out with something like this. When you look at code review as an activity, it operates on two levels:
- Don't overthink it. It's a relatively stock tactic for improving code quality.
- It's also a game of king-of-the-hill for a very small hill, organizationally speaking. Poker without table stakes, played for buttons. (It's what the Gervais Principle would call the loser language of Gametalk).
To use my lexicon, code review tends to be the journeyman idealist crucible for establishing a pecking order no one outside of the dev group cares about. But within the dev group, it determines whether you're one of the popular jocks or one of the, well, nerds, I guess...? (I don't know how to make that analogy not ironic)
Within the insular culture of team code review, power dynamics establish themselves. On the most basic level, this will hew to seniority by default. People that have been with the company longer have more clout (for some dubious definition) than newbies, which means that seniors "do" code review "to" newbies, typically.
Preferring Bluntness is a Power Grab, While Preferring Subtlety is a Power Cession
When you add the subtlety-directness cultural concern into the mix, you smack up against a concept known as power-distance. You can read more about this here, but for our purposes, understand that these different cultures have different outlooks on organization structures, power, and behavior. "Direct" cultures tend to assume that everyone from CEO to IC will speak his or her mind. "Subtle" cultures strongly believe that role dictates behavior, deference, and social interactions.
So, when you are "blunt" in a heterogeneous group, you're not just "speaking your mind, man." You're making a statement that you're in charge. And subtlety makes deference a self-fulfilling prophecy.
In other words, this choice of "nice or blunt" isn't just about how you like your feedback. It's you staking a claim to your power or lack thereof.
Your Approach to Code Review Can Have Organizational Stakes, Even Though Code Review Doesn't
Now, as I've said, this is power on a very small scale. It's the equivalent of a Stack Overflow badge in your organization. Notice that no VP comes along and gives you an office because you have the best Linus Torvalds imitation at code review time.
Being blunt is to lay claim to "senior" and being subtle is to lay claim to "junior" (I hate this term), by default. The organization may even make this prophecy self-fulfilling at some point. But recognize that, from any modestly sized organization's perspective, senior developer and junior developer are the same thing.
After a bunch of years and contentious code reviews, you might carve out a bit more money and bragging rights on the developer track. But, if you're an opportunist or aspiring opportunist, that's not what you're after. And that's okay. Your behavior and approach to code reviews can still matter -- just not in the way they matter to journeyman idealists.
You Don't Want to be Greg House and You Don't Want to be His Buddy Wilson, Either
I mentioned Linus Torvalds, who I kind of think of as the Gregory House of the software world. The premise of Gregory House is that he's a doctor and such a transcendent genius at diagnostics that his hospital excuses all manner of terrible, liability-incurring behavior. He's a drug addict, flaunts the law, listens to nobody, etc, but he's just so darned good that the great meritocracy of the world deigns to forgive all of this that it may bask in his reflected glory.
But the world doesn't work this way, and certainly not the corporate world. Greg House wouldn't last 3 days in a hospital, and Linus Torvalds himself doesn't need to, since he's more akin to a performance artist than a corporate denizen. Companies won't tolerate this archetype when the hijinks outweigh the benefit.
Now step back and think of the blunt code reviewer. To the ascendant and placed opportunists, this archetype looks like a journeyman idealist fantasizing about being House/Linus. "I don't have time to be polite about my feedback" sounds comically self-righteous in this context. The implication being, "you're like Linus Torvalds, but without the achievement."
On the flip side of this, you have House's foil the sensitive pushover Dr. Wilson. Wilson is definitely the king of compliment sandwiches and the avatar for subtle culture types coming in at the bottom. And you don't want to be that, either. If you let House/Torvalds stomp all over you, you're not destined for the halls of power. Opportunists maneuver (journeyman) idealists -- they don't quail in front of them and ask to be transferred.
The Human Approach: Use the Approach the Reviewee Prefers
Let me offer a brief interlude in talking about how this affects your career in Machiavellian terms. What you should probably do, as a decent collaborator and human being, is get to know your coworkers and do what they prefer.
Invariably this debate features a lot of "this is what I like, and so we should all as a group do what I like because what I like is best." When you grok the cultural backing, this becomes unforgivably childish. When you get to know others, including their personalities and cultural proclivities, you can start to offer feedback to them in the terms they prefer, be those blunt or complimentary, direct or subtle. That's the decent thing to and the most likely to get results.
The Realpolitik Approach: Solve Code Review Squabbling So Opportunists Don't Have to Care
Now, let's look from a career perspective. Do you think anyone outside of your group cares whether you use the composite pattern to manage your navigation menu? Let's go one step further. Do you think anyone outside of your group cares about whether or not you furnish compliment sandwiches while discussing this detail?
This stuff is not important to a business. Full stop.
So when you argue about code, code reviews, and whether you should be blunt or not, anyone who you bother with those arguments will view both parties as losers of the argument. It's a policy of mutually assured destruction in terms of opportunist cred. Whether you're going to a manager about someone or them about you, or whether you're both just duking it out, you both lose.
So your approach to code review should be first, the human one, for diplomatic reasons. But it should also be to create a human code review hegemony within your group, and to do so visibly. Nobody outside of the dev group cares about this, so you'll notch yourself a critical win if you visibly make it a non-issue for everyone.
In the broadest of terms this might mean consensus building or it might mean something as simple as automating the whole code review process. Whatever it is, though, it will come in the form of removing an opportunist's headache.
In the End, The Right Answer is An Algorithm -- Not a Style
If you take one thing from this post, I hope it's a recognition that there isn't a right answer to the question of code review (or collaboration in general) feedback style. Even if you magically had one for your group somehow, it wouldn't translate. Groups are comprised of different people, from different countries, different backgrounds, and different proclivities. And that's not a platitude -- we are literally raised from birth to have different takes on how code reviews should go.
So stop trying to decide which is the right strategy, and start recognizing that the right strategy is one that you tune according to your audience. And recognize that the right strategy is one that stops this issue from wasting people's time and filling their days with an analog of adjudicating how children fight over toys. That recognition is good for your day to day happiness, and it's good for your career.
Top comments (15)
nice post. I am an Italian living in Germany. I can tell that I never really like the "italian way" of telling things: this subtleness where you always try to be kind, friendly, funny often ends up in utter hypocrisy and more often in sarcasm. Of course, the first weeks in Germany surprised me. They are really blunt and direct, but I immediately liked it. If something is wrong - and I am not talking about coding style - but implementation details ( memory leaks, performance issues - sometimes code readability - why can't you just say it directly - without false compliments and the sandwich stuff.
Don't get me wrong. Being blunt does not mean being offensive or aggressive. I understand that there are cultural differences - as an Italian I gesture a lot - and I am probably louder as well - therefore when discussing code with a Japanese colleague I inevitably look and sound more "passionate" about my stance. ;-)
Kindness and politeness is key. everywhere. but it should not hinder the message being transferred. If I say "I really like the effort you put into this bla bla bla, it might be my impression but this could be done differently, and i would have probably have written this" - not only i need more time than writing "It would be better to do this, please refactor/change it" but the reviewee might just think - well it's his opinion. i will keep it in my way.
At work we do code reviews within the pull/merge requests in gitlab, so normally we just write a couple of sentences in the points we want to have fixed / changed. kindly but firmly.
What i always do though, is pointing out with positive a positive comment some parts of the code which i like or find particularly clever. They are not required during the PullRequest - but i guess that can make the code review less "demotivating" ( in the end we are just pointing out mistakes - that's what MR are made for)
Something that i like to point out is also - use linters so that the code review has nothing to do with style and formatting.
I'd distinguish between the "like the effort" bit and the "might be my impression" bit -- the former is harmless but the latter waters down your stance. And ideally, there'd be some mention of why the proposed alternative is better.
But I agree with your larger point. It's nice when people get the point. Belle parole non pascon i gatti...
I actually really enjoy seeing what I've missed, even if I might feel dumb later. It's like seeing a magic trick get revealed. But I've seen folks dread code review who were nevertheless very good devs. Everyone's politeness meter is calibrated differently, I guess.
despite being italian i never heard that (it's probably regional :-) very very nice!
lol. my father's from Avellino province but no one in my family says it either. I saw it online and it seemed cute and half-relevant :P
Great post. I personally feel that bluntness saves time and it's definitely necessarily to be critical. Adding a few pleases/polite terms helps to calm down the tone of post which allows messages to get across without sounding too bossy/offensive...:)
I agree. In particular I think that supplicating-by-default can be very problematic when folks aren't expecting it.
Ideally, among knowledge workers, "power" (though I prefer "standing") follows from correctness. If that doesn't happen during code review, the only good option is to leave for a place where it does. Maybe it's naive, but I think a focus on correctness is how you break out of the politics.
At best bluntness should be tempered by:
I try to be right and nice, in that order. And I try to know when I'm not sure, or when it's a matter of taste.
Folks complain about the tenor of Stack Overflow, but the only time I see people get chewed out there is when they're very confidently wrong. IMO such reprisals are useful for maintaining a culture of accuracy.
Unfortunately no one tells stories about when someone isn't a dick. We've got a big pantheon and I'd argue that most of its members are fairly unassuming. I've never read about Kay, or Steele, or Hickey telling someone they're "so stupid it's a wonder they figured out how to breastfeed." (That's the cleaned up version.)
That was my first thought. If you know the person, use the style of communication they prefer. But be nice. And be right. Ideally, explain why you think you're right, and have the integrity to also say why you could be wrong.
I'm trying to resist the urge to write a novel on the subject of office politics, since it's both something I've talked a lot about on my blog for years, and, in its current corporate incarnation, the thing that eventually drove me out of employment and into working for myself. At any rate, I'd say the fatal flaw experienced by most groups aiming for "correctness as currency" would arise from issues where correctness is either unknowable or else non-existent due to subjectivity.
So, at any rate, I don't take what you're saying as naive at all, but rather the seed of a good mode for interaction. Because if correct is (currently) unknowable, that should result in extremely productive discussions of "this call seems subjective, so how could we run an experiment and measure it?"
For me, that approach tends to eliminate a lot of discord, in the polite vs. blunt arena, but in terms of human interaction in general. "Is this a good blog post" is a question that can quickly become a terminally stupid shouting match between respondents. But, what if you say, "good as measured by what -- time on the page, reader engagement, etc" you can then start to establish outcome-oriented metrics and have more productive conversations.
My book Programming Without Anxiety will address code review. Some important bits here:
Explain the generic idea as well
Whenever pointing out a specific thing, describe the generic underlying idea as well. By adding context, the author doesn't feel scolded, rather understands the intent towards a broader rationale. (And might dispute the rationale, not the specific comment).
For example, "Make this method final." can be extended with "Generally, prefer making methods in this class final, as we want to avoid accidental overriding. We had bad bugs about that in the past.".
Single comment per category
Add only a single comment about a single category of issue, and have an agreement that authors should fix all relevant issues without asking.
For example, don't write "Remove trailing whitespace here" on 20 lines. Add only on one line, maybe saying "Remove trailing whitespace - other lines too" initially.
Style guide
Have a public style guide, so authors diverge less. Keep it updated with common mistakes.
Ask for correction first
If a piece of code looks quirky or impenetrable, don't tear it to pieces. Rather start by commenting "I'm confused by the control flow here. Could you make it cleaner or comment?" or "This looks complex, please simplify or add more tests.". Often the author knows a piece of code is quirky, and a small trigger nudges them to fix it up.
If the code still comes back complicated, then make specific improvement comments.
If you like these and want more, subscribe to (or better, prepurchase) my book!
I have become disgusted with how people take criticism over the past few years. It gets worse and worse, to the point that simply saying:
(actual post)
Some places -- I call them dens of suck-ups and sycophants -- that's now treated as a ban-worthy post as if I'd used every four letter word in the book mated to Carlin's "Seven Dirty Words". Some will even say "that's not constructive" -- what the bloody blue blazes is "more constructive" than providing a list of everything that's WRONG?!?. What are we supposed to do? Slap the rose coloured glasses on their heads and whisper soothing-syrup words in their ears as they drive off the cliff?
... and why? To maintain the status quo 3i of web development: Ignorance, Incompetence, and Ineptitude.
Mind you, I don't use "ignorant" as an insult. It just means you don't know. It's only when you're told and continue to willfully do things badly that it becomes the other two.
There's this "wah wah, somebuddy usededed teh hursh wurdz" attitude that's flushing this entire industry down the toilet, allowing every two-bit dirtbag scam artists peddle their snake oil, and silencing anyone who dares speak out against it.
How DARE anyone get upset by sleazy scams. How DARE anyone get upset by poseurs not qualified to work with the underlying languages like the know-nothings who created and maintain bootcrap. How DARE anyone make waves, rock the boat, or be "confrontational" over bad practices, nubes being led down the garden path to failure, or any of the dozens of other dishonest bald faced LIES that make up the majority of "new" development practices of the past ten to fifteen years.
No, that might "upset" someone. Heavens to Betsy, nots thatz.
But really that's what this "It's not how you said it" or "if you can't say anything nice" rubbish boils down to, controlling the narrative and forcing a language of control. It's part of the "bandwagon" process of propaganda, and is why those singing the praises of so much of the outright incompetence in this industry follow up quickly with transfer, glittering generalities, plain folks, name calling (and not the crass direct type I just used; But to understand that, you have to recognize that "easier" is name calling....), testimonial, and of course card stacking. That all seven? Yup, all seven propaganda techniques. Learn them, recognize them!
Can't argue the facts, so attack how they were presented whilst screaming "wah wah, is not" like a petulant seven year old. The bread and butter of "modern" discourse. Nothing more than lame excuses to silence anyone who doesn't let themselves be hammered into that round hole.
Though I freely admit the closer I get to the big five-oh, the less and less I give a flying purple fish about what people think about how I say things. My internal censor was killed off twenty years ago, but now I'm just so frustrated with the endless streams of lame excuses for lamer development practices that the kid gloves are off.
Wow... you tell 'em!
This is a great article! I'm here because I work for a SaaS company that builds an automated code review tool. Being totally new to the developer industry I couldn't figure out the pains of communicating code reviews. This is great learning! Thank you Erik.
While I agree that a mismatch between subtle vs direct communication styles can cause a multitude of problems*, I think you may be mixing in an orthogonal behavior axis: cooperative vs combative.
For example, let's assume a review for a bit of code that needs work. I tend towards blunt/cooperative, so unsurprisingly I'm not really good at picking up when someone's being subtle/combative, so these examples will probably be a bit exaggerated. Hopefully, they'll be close enough to get the idea across.
blunt/combative
Characterized by abusive language, this is the one we generally think of when talking about abusive behavior in reviews. Linus Torvalds is an infamous example of this communication style.
subtle/combative
The best description I came up with for this one is "passive-aggressive sniping".
blunt/cooperative
This one is the one I tend towards by default, so I'm most familiar with it's strengths and weaknesses. Basically: direct, polite, and very clear about what is expected.
Often this involves including code snippets, on the grounds that, if it's worth bringing up, it's worth taking the time to suggest something concrete.
If you tend towards this style, be careful to use open-ended phrasing and acknowledge the limits of your certainty frequently, or you'll run the risk of sounding like a pushy know-it-all to folks who prefer subtle communication.
subtle/cooperative
Polite and (if you prefer direct communication) a bit on the vague side, but still providing enough information for the reviewee to fix the issue.
* This can cause trouble even when everyone on the team comes from the same area. For example, in the USA men tend to be socialized to favor direct communication, and women tend to be socialized to favor subtle communication. This causes about as much trouble as you might expect, if people don't actively work to mitigate the mismatch.
Code review should be "Lead by (code) Example" rather than blabbering lot of concepts or jagrons .
Code speaks a lot , Words doesn't.
I blogged about my take on code reviews a little while ago, so here's the obligatory link to my 2 cents mcod3.wordpress.com/2019/04/24/pee...
Some comments may only be visible to logged-in visitors. Sign in to view all comments.