Once more, with feeling: A radical approach to code review

I was lucky to be in Dropbox’s headquarters when Kim Scott came to speak about her (then) new book “Radical Candor”. Dropbox was nice enough to hand out free copies (yay! free books!) so I actually ended up reading most of the book on the flight home.

Even though the book was written with managers in mind, I found myself referring back to it pretty often. Its insights proved quite useful in my daily work as a software engineer, especially in processes which require written or in person interaction with our co-workers. A little bit of Radical Candor can totally change how we do and react to code or design reviews, driving better results and higher satisfaction.

Radical Candor: Some Background

Before I share how you should use Radical Candor while creating software, it might be helpful to explain what Radical Candor is. I will try to keep this section as short as possible, since it’s just setting up the background for how to use Radical Candor in the context of software processes. If you’re already familiar with the framework you can just skip to the next section (titled Radical Code Review).

Radical Candor is about giving guidance that’s kind and clear, specific and sincere (source here).

To achieve this goal you must care personally and challenge directly.

Care Personally

Remember what HR stands for? We might not like being labeled as “resources”, but the “human” part is something we should take note of. Being human, we have feelings. And those feelings affect our work. Some cultures might view emotions as an unprofessional design flaw, but truly — it’s not. It’s part of the package — you can’t expect people to be passionate, engaged and excited at work, while at the same time expecting them to leave frustration, worry and awkwardness at home.

Ignoring the human feelings part of the equation leads to unhappiness. Unhappiness leads to alienation and cynicism. Alienation and cynicism lead to bad results and attrition.

You don’t have to be BFFs with everyone at work, but acknowledging feelings and showing that you care is an essential part of working together. As an added bonus, it’s part of being a decent human being, without a team-work-collaboration end in mind.

Challenge Directly

We all try to avoid saying things that are uncomfortable. Some cultures more than others, perhaps, but everyone will try to avoid awkward and difficult conversations. If we do manage to pluck up the courage to give some tough feedback, we will try and sugar coat it in a way that will soften the blow.

However, when we are not clear and direct we are running the risk of our message being misunderstood. And in the long run, avoiding hard truths is a favor to no one. Without being clear about what needs to be improved and what should be built upon and reinforced, people can’t improve and grow. Bad results and frustration follow.

The combination of these two dimensions is what defines whether we will say anything, if we will say the right things, and if what we say will be received well and be effective in the long run.

Radical Candor is the quadrant we’re aiming to be in: where you care personally and challenge directly. You have taken the time to establish that you care, understand the person in front of you, and give them clear and actionable feedback. They might still get hurt or feel awkward receiving feedback, but are likely to appreciate it and modify their behavior accordingly.

Obnoxious Aggression is when you give feedback that needs to be given, but don’t take two seconds to show you care. Saying things like “you are an idiot” is not productive. Even praise can be obnoxiously aggressive when saying things like “Wow! You managed not to fail!”.

This is better than not saying anything, but in the long run causes your colleagues to want to stay away from you. Being around a jerk is not fun, and very demotivating. People won’t want to listen to what you say and your message will not get across. Even if essentially, what you’re saying is true.

Manipulative Insincerity is when you don’t care enough to offer real feedback. You just give empty praises because it’s easier than having to challenge them: “Great presentation! Really liked it!” and walk away even though the presentation was terrible, maybe even making fun of them behind their back.

You might tell yourself that you’re doing it because you don’t want to hurt their feelings, but really it’s to protect you from having to have a hard conversation or risk not being liked.

Ruinous Empathy is when you put too much emphasis on playing nice and getting along, and end up avoiding conflict altogether. Praise in this quadrant is empty “good job”, and criticism is non-existent. This leaves people in the dark with regards to what they’re doing well and what they should improve, until it explodes in their faces and are fired (or the rest of the team quits because they’re tired of covering up for the low performers). It’s not even their fault — no one told them what to improve, so how could they?

If I had to summarize Radical Candor in two sentences, I would say:
Radical Candor is not about telling the truth no matter what.
Radical Candor is about building relationships where the truth is welcome in order to get better quality results and improve personally.

Radical Code Review

Why do we even have code reviews?

Better quality results

Code review, when done right, keeps code quality high. Another set of eyes to review naming, structure, flow and tests coverage ensures the code is logical, clear and readable. Which means it’s maintainable in the long run. Which means it’s faster to develop new features and is less error-prone in the future.

Improve Personally

When someone reviews your code, they point out things which could have been done better. When you review someone else’s code you get a chance to see what they’re working on, gaining exposure to areas in the code you might not have worked on as well as someone else’s style and way of thinking. Either way, there’s much to learn and grow from the experience.

Code Review Bites

You might be thinking “Fine, I get it. Quality results, improving personally. But where’s the Radical Candor part? It’s just a code review!”. Let me tell you: when there are people involved, it’s never “just a code review”.

Have you ever felt a twinge of embarrassment, a feeling of inadequacy or frustration during code review? Or maybe even stronger feelings like anger or hurt? If you have, you’re not alone. I’ve certainly felt this way. I’ve also asked around and I know other people have too.

In fact, that when I couldn’t remember what to call “the person who wrote the code being reviewed” people suggested “victim” (twice!), “poor bastard” and “potential idiot”, suggesting that code review is not considered a pleasant activity by many who’ve been through it. By the way, I decided on “author”.

It’s not just code. It’s our work. And we’re emotionally invested in it (remember? feelings are part of the human package). Then someone comes in and casually comments that our code doesn’t make any sense, the tests are all wrong, and we should rewrite the whole thing. Right before the weekend, probably. No wonder we get upset.

So, we’re upset and grumbling away at our computer, typing furiously, making the changes they requested resentfully. Maybe slacking a friend to complain. Maybe we decide to ignore the comments entirely, as they’ve obviously been given by an idiot who knows nothing and isn’t worth our time.

See much quality results or improving personally in that last paragraph? I don’t think so.

Once more, with feeling

I’m sure no one sets out to crush a person’s spirit when reviewing their code. At least I hope no one does, because that’s a terrible, terrible attitude. I assume they just wanted to focus on getting quality results, and forgot the human side of things.

For the purpose of this post I assume you have the technical part down, so this section will cover how to communicate the comments you give in a radically candid way:

Remember that a person wrote this code: The author put a lot of mental effort and time into it and they want to be proud of their work. Never ever say that the code is “bad” or some other generalized judgement of their work. Certainly don’t even think of calling them anything personally judgmental, like “stupid” or “lazy”. This hurts and will shut them down to any further feedback.

Your mindset should be a teaching/learning mind set. Don’t show off your superior knowledge or use it to bully someone else. Actually, don’t assume you have superior knowledge. It doesn’t really matter either way, you can learn something from anyone.

Don’t hold back your technical comments either — your perspective matters! If you hold back you’ll be missing a teaching moment and the overall code quality. If you think something can be improved, comment away!

Approve with comments

Each code management system has their own process for code review and approval. Some have a structured process for choosing reviewers, commenting and approving. Others have a free-form discussion where an agreement is reached by getting LGTM approval from all the participants.

If you have a formal process you will normally have an option to block or approve the changes. At Dropbox these are labeled request changes or approve, so I’ll go with that terminology, though I’m aware other systems use different terms.

In the Obnoxious Aggression quadrant, we have request changes. Basically, any blocking state says that you don’t trust the author to make the necessary changes before they push their code. If you feel it’s absolutely necessary to block — i.e. the change poses a security risk or introduces a major bug — go ahead and use it, but remember to clearly explain why you are choosing to block, to mitigate the perceived distrust and disrespect which causes the author to be less receptive to your comments.

On the author side of things, being blocked does not give you permission to send your changes to be reviewed by someone else, no matter how mistreated you feel. But such things have been know to happen, and that’s not just obnoxiously aggressive towards the reviewer, it’s also manipulatively insincere towards the new reviewer, who doesn’t even know they’re overriding someone else’s decision.

In the Manipulative Insincerity quadrant, we have those who comment without approve. You make a few comments, but don’t make a decision whether to block or approve. This leaves the author confused whether the comments are important or just nits, while still leaving them blocked.

This is of course a legitimate course of action if you have questions or don’t feel like you have enough domain knowledge to approve, but as before — you should think about how this is being received on the author’s end and be clear about your intentions and why you’re not approving. Just saying “Hey! I added some comments but didn’t approve because I think Alice is the expert in this area” or “I didn’t quite understand what you did here so I left some questions I need answered before I can approve” goes a very long way to diffuse frustration on the other end.

In the Ruinous Empathy quadrant, we have those who approve without any comments, even though they have comments to give. Sometimes they feel their critique will hurt the author’s feelings or would take too much effort to explain. This prevents the author from learning from their mistakes and improving, while also hurting the quality of the code. Don’t hold back, comment on what you think can and should be improved, that’s the right thing to do professionally and personally.

The Radical Candor way is to approve with comments. Approving with comments tells the author that you trust their judgement to respond to the comments and make the necessary modifications before pushing their changes. The author is unblocked, but is able to learn from the comments you’ve written and the code quality is maintained. Everyone wins.

Unfortunately, we don’t have approve with comments as an official state in our system, but we can approve after we’ve added comments. It’s better than nothing, but it can be confusing and easy to overlook. I’ve had a few close calls when I noticed comments during the push process and had to cancel it mid-push. I’m planning to follow up on this and try to add an approve with comments state which unblocks but is very clear and direct about the expectations before pushing the changes. Maybe during next “hack week”.

Radical Design Review

Small tasks can be covered by code review after the fact, but what about larger tasks which require design and planning?

Enter the GSD (get stuff done) circle, which describes a framework for… well… getting stuff done as a team.

Different workplaces have different team structures and processes, but I think it’s safe to assume that each large task has a DRI (“directly responsible individual” — whether it’s a manager, tech lead, architect or team member) and a team of non-DRI people collaborating together to get the work done. The GSD circle provides some excellent guidance for how to lead without authority, which is often necessary when the DRI is not a manager or needs contributions from other teams to complete the task at hand.

Sometimes we spend too much time on design and planning, and end up with an over-engineered and perhaps wrong solution. Sometimes, in the interest of saving time, we rush into implementation without a proper design or planning process and end up wasting more time on rewrites than we would have on proper preparation. As I see it, the first few steps on the wheel which cover the design and planning process (listen — clarify — debate — decide — persuade), if executed well, can save a lot of time in the short and long run.

Learn

Remember that DRI doesn’t stand for All Knowing Genius. For one, the acronyms don’t match, but also — in reality — no one person has all the whole truth in their heads, even if they are tech lead or a high level experienced engineer. Designing software is not a solo task. The reason we even have a team is so we can benefit from everyone’s knowledge and unique perspective.

The DRI should assemble a diverse yet relevant team. Don’t waste people’s time if they don’t have anything to add, but don’t exclude someone just because they’re not going to end up writing code for this particular project. Probably 5–7 people is ideal: They should be with varying seniority and, if possible, include some people from other teams with relevant knowledge.

As a team we need to be sensitive to everyone present: Some people are naturally quiet. Others lack confidence to speak up. Some people dominate the conversation. Junior engineers might be scared to disagree with senior engineers. Remember that good ideas are not necessarily correlated with communication style or seniority — the quiet ones might have the best ideas, and junior engineers might have a fresh perspective the senior engineers have overlooked.

All points of view should be respected, never dismissed offhand. If we don’t respect different opinions (whether we just don’t agree or they are objectively wrong), that person will shut down the next time and not offer any more ideas.

A culture of listening means that everyone will be able to contribute to the discussion and provide valuable input, which will result in a diverse set of good ideas.

We don’t need a manager in the room to do that, but a some discussion moderation might be in order. This could either be the DRI or a role assigned at the beginning of the meeting (just like a note taker).

To make this meeting really effective, the DRI should share some background about the problem trying to be solved, and participants should come prepared. Since people tend not to prepare ahead of time, presenting a short overview for the first 5 minutes of the meeting might be a good way to get everyone focused and synced. An hour total should be enough for raising a number of ideas and setting them up for the next step.

Clarify

Once we have a set of ideas, we need to process them into alternative options which are understandable by the entire team and other decision makers. Sometimes there can be a lot of argument and debate around an idea which no one around the table actually understood!

I believe this part should be done offline, with each participant who had an option they want to be considered for design choosing a partner to bounce the idea off of. If it makes sense time-wise and team-wise, the DRI could be everyone’s partner. The partner should challenge and question the idea until the design is clear to them both and they can define the basic concepts clearly (API, code structure, interfaces) and what differentiates it from the other options.

If at all possible, you should try and find a short title which captures the essence of the design option. This will make communicating about it afterwards much easier. Something like “chocolate factory” is easier to discuss than “a family of cocoa objects which is created via a unified interface”. In addition, in the future, when making other, smaller decisions, it will be easier to identify if they are aligned with the “core” of the option or not.

This step might take a few iterations, as sometimes ideas need some time to “sink in”. I know I’ve had ideas that just seemed intuitively right to me, but it still took a day or two before I could really explain why they were the right solution to the problem. All in all this process should take one or two hours over a couple of days.

When done, the solution should be summarized into a common design document for all the participants to review. If they have questions, they can comment and the design option can be clarified even further before the next step.

Debate

Once you are clear about the different options, it’s time to debate. Get everyone back in a room and discuss the pros and cons of each option.

People tend to be emotional about their ideas, so tread lightly: You are debating on the merit of the idea, not the person who raised it! You shouldn’t be arguing to win, you should be debating so that the best option wins. If the discussion gets too emotional the moderator can point it out and take a pause if necessary. You can also try asking someone to argue someone else’s position, that forces them to think about the merit of the option while alleviating part of the emotional attachment.

Sometimes people buy in to an idea too soon: Maybe because it sounds good or easy to implement, maybe because the presenter is charismatic, likeable or in a position of authority. It is the team’s duty to disagree! If no one disagrees, designate a devil’s advocate — there can’t be a good decision without debate.

Take care to set the expectations clearly to what is actually going on in the meeting so that people understand what they’re trying to achieve — the mindset of debate is different than the mindset of decision making. While debating people can see both sides of an issue, change positions if they want to and consider pros and cons of different approaches: speed, simplicity, maintainability etc. You can try and combine different approaches to achieve one uber-option. You can still play around with different ideas. At some point you have to switch into decide mode, you have to weigh the cost and benefit of each option as it is and move forward.

This can be a debate only or a debate & decide meeting, but whichever you choose, you need to make sure to allocate a separate time for deciding and be clear about when you are transitioning into decide mode. There’s nothing less efficient than trying to make a decision when some people are still debating.

Decide

Since the DRI is not always the most senior engineer or the manager, it’s worth mentioning that it should be clear to everyone who makes the final decision. This doesn’t make the DRI a dictator, they must consider the discussion and base their decision on that or face a total breakdown of trust and collaboration (plus all that wasted time on listening and debating…). But since they are the one’s who are accountable for the results — they must have the final say.

Common design review process mistakes
We sometimes neglect the listen-clarify-debate part of the design review process and jump straight to decision making. When we treat the design review as a decision making meeting, kind of a rubber stamp — we’re missing the point. Also, when expectations aren’t aligned and someone challenges our recommended option or raises new questions, we resent it — as we’re sure we’re at the decision stage, while they are at the review to debate.
A useful practice to eliminate friction and reduce time wasting is to get all the stake holders involved as soon as possible. Once you’ve done the initial listen-clarify-debate in the 5–7 limited team, get the other reviewers (from other teams and groups) up to speed and get their feedback as soon as possible. Be open to listen to what they have to say, they’re there to offer their perspective and knowledge. Listen-clarify-debate with them as well, and arrive at the final review prepared to make a decision.
I’ve seen a tendency to skip some or all of these meetings altogether in the interest of saving time and just use a shared document and comments for discussion. This may sound effective, but it’s not really — it makes all the problems of the meeting way worse — group think, dominating actors, people not saying what they really think. Without open discussion it’s much harder to raise and refine innovative ideas!

Persuade

Disagree and commit — Andy Grove, Intel CEO

I don’t wanna! — My 3 year old

Disagree and commit is an important skill to learn. We can’t always get our way and sometimes we have to execute on decisions we don’t agree with or understand. However, our inner 3 year old is always ready to come out and say “I don’t wanna”, and if anything goes wrong in the project, we have a little “I told you so” to go with it.

Executing when there are people on the team who feel like this will be hard. Every small decision will be second guessed, and every hiccup or setback will be viewed as an opening to revisit the entire decision and start over.

There usually are a few people on the team who need to execute on your decision but weren’t part of the decision process, You can’t just expect them to disagree and commit without understanding. Once the design review is over, the decision should be communicated widely. A short description or a reference to the document which contains all the options, and an explanation what makes the chosen option the best one will usually be enough. For those who have additional concerns, try to be patient and figure out what the root issue is:

• They might disagree with the decision because they weren’t there to see the process and don’t realize the option they think would work better was discussed and dismissed. Explain why the idea they’re trying to push for wasn’t chosen.
• They might be opposed to the idea because it sounds like a lot of work and they’re tired and burnt out. That’s not a good place to be in emotionally, and should probably be addressed in general, but in this context — explaining the decision won’t do much. You need to explain why the work is worth it or how it will save time in the future, and what support they can expect while working on this project.
• They might be resentful simply because they feel they were left out of the decision making process itself. Apologize if necessary, and make an effort to include them next time. If there’s a good reason why they shouldn’t be included (and it better be good), tell them directly and give them time to accept that they’re not going to get what they want. Ask if there’s anything else you can do to make them feel included (e.g. invite them to comment on the design review document instead of participating in the meetings).
• They might have some other reason for dissent. Take the time to figure it out and address it directly.

Not everyone has to be in love with the idea, but you do have to make sure they’re aligned and will go with your decisions.

Execute

This is the time to stop attending a lot of preparation meetings and just do the work. You should still keep the team aligned on progress and smaller decisions through regular meetings or informal communication, but this is the part where you double down and write a whole bunch of code.

Learn

When we’re done executing (some specific task, we’re never really done executing…) we need to take a step back and review how it went. Celebrate the success, review what went well and what could be improved and try to get better for the next time. Retro meetings are a common practice and shouldn’t be skipped. Candor and openness to different point of views are important for retro meetings as well — if we don’t feel safe to share our opinions we won’t be able to keep doing what went well and get better at what went not-so-well.

The importance of being agile
I’m sure there are a lot of engineers reading through the design review section scratching their heads and wondering how this fits into the agile framework. This process sounds way to time consuming for one-two-even four week sprints. I agree, it does.
One answer to this very understandable concern is that if a team has built trust in each other and in the process and are accustomed to working this way, the GSD wheel can turn faster, and the entire cycle can be completed in a few days.
The second answer is that this process should be reserved for Type 1 decisions which are hard and costly to reverse. For these decisions you should take a reasonable amount of time needed to make them correctly, even if it “wastes” a large chunk of the sprint.

Recap

• Radical Candor is about building relationships where the truth is welcome in order to get better quality results and improve personally.
• Radical Code Review is about giving solid technical feedback, unblocking as quickly as possible, and being respectful by approving with comments.
• Radical Design Review is about making the best possible decision by giving everyone on the team a voice and getting everyone on board with the chosen solution.

Way to go

I’m really impressed you made it this far, reading a post this long is quite a challenge and not to be taken for granted.

See what I did there? That was a radically candid compliment — clear and specific. And here’s something actionable for you: Clapping or leaving a comment would be extremely welcome!

Special thanks to Erel Sharf for pushing me on the design review section, this post would be much shorter without him :)

---

Originally published in Medium on January 6th, 2020