#159 - Leveling Up Your Code Reviews from 'Good Enough' to Great - Adrienne Tacke

[00:01:46] Episode Introduction

Henry Suryawirawan: Hello to all of you, my friends and my listeners. Happy New Year 2024! This is our first episode of the year, and welcome back to the Tech Lead Journal podcast, a podcast on technical leadership and excellence.

If you haven’t, please subscribe on your favorite podcast app to get notified for new episodes. Tech Lead Journal also provides bite-sized contents on LinkedIn, X, Instagram, YouTube, and TikTok.

My guest for today’s episode is Adrienne Tacke. Adrienne is a software engineer, keynote speaker, and the author of the upcoming book “Looks Good To Me”. In this episode, we discuss code reviews and why it is an essential part of the software development process. Adrienne discusses the importance and benefits of code review, the common code review workflow and the different roles involved, how to provide effective code review comments, and why we should leverage on code review tools and automation. She also provides tips on how to speed up our code review turnaround time.

I hope you enjoy listening to this episode and learning best practices on doing code reviews. Remember to share it with your colleagues, friends, and communities, and leave a five star rating and review on Apple Podcasts and Spotify. Let’s now go to my conversation with Adrienne.

[00:03:06] Introduction

Henry Suryawirawan: Hey, everyone. Welcome back to another new episode of the Tech Lead Journal podcast. Today, I have with me Adrienne Tacke. She’s actually writing a book, which is something that I look forward to reading, which is titled Looks Good to Me. I mean, if you know this term right, I’m sure you can associate that with code review. Maybe we’ll ask her what does look good to me means. So Adrienne, welcome to the show.

Adrienne Tacke: Thank you, Henry. Thank you so much for having me. I’m excited to be here.

[00:03:31] Career Journey

Henry Suryawirawan: Right. Adrienne, I like to ask you to share a bit about yourself, right? If you have any career highlights or turning points that we can all learn from.

Adrienne Tacke: Sure. I think the top three highlights I’ll share was, in the beginning I was never really introduced to STEM as most people would. So I didn’t really have any family members who coached me or exposed me to that. My only exposure was through playing my own computer games, rather, like RCT 2 or Age of Empires II. And that’s a very important thing I like to distinguish because a lot of people think that getting into a career, you must have had this love for STEM or coding or doing these types of things from a very early age. And while that’s helpful, for me, I didn’t know that I wanted to be in this type of career until college when I was a student help desk technician. So that’s probably the first turning point was I didn’t even get that job because I wanted to. It was the highest paying job at the time for university. And it was only there where you would problem solve, kind of help people break down their problems and try to help them was where I said, huh, this might be an avenue that I want to go into.

Fast forward to probably middle of my career, I was a .NET developer for small companies here around Las Vegas where I was based. And this is where I started to learn that not a lot of people dressed like me. So, you know, you have this stereotype where it’s like, you know, you dress casually or like tech people don’t like to dress up. I don’t know, that’s probably false now. But at least at the time during my career, nobody dressed up in heels. Nobody wore dresses. No one wore skirts, no one did their hair like I did. And so that was a big turning point for me to kind of share. And so I started to share that on an Instagram account, where I’ve actually grew fairly large without intending to. But I wanted to show other women specifically and other Filipino women because there’s kind of this notion and push to go into the medical field of, hey, there’s actually a different avenue for you to go into. So that was a big part, was to share that you can be feminine and be technical and you can go into the technology field if you don’t want to go into the medical field, like I didn’t.

And then the last turning point I’ll share was, later in my career, I went to a technical conference and I was listening to a talk. And the person who was sharing or giving the talk was really, really bad. I mean, I wasn’t really learning anything. The slides were just bullet points. I started to wonder, how do you actually get up onto these stages, cause I felt like I could do a better job of presenting than the current person I was watching. And so that’s when I kind of learned, oh, there’s this whole process of applying, you know, submitting talk proposals to these conferences, organizers choose the best ones that fit their program. And then that’s how you actually can become a tech speaker. And so that was a very big turning point because I was like, okay, let me just try this out. And as a new person, someone who’s never been heard of on the speaking stage, the advice they tell you is to apply to as many as you can, because, you know, if you get rejected, a lot of times at least you have more chances.

Well, at that first time I actually got accepted to seven conferences. And so that was very, very interesting. And that’s kind of gotten me to where I am today as a Senior Developer Advocate. I wasn’t expecting to transition into Developer Relations, uh, nor was I expecting to be, you know, to actually really like speaking on stage. I’m usually a very quiet and introvert, don’t like talking to people at cocktail parties, so to get up on stage is something that’s very different for me, but I actually really, really love it.

Henry Suryawirawan: Thank you for sharing your story, I think it’s really interesting. You highlighted three different things, right, which I think many engineers could actually relate to. I think the key learning for me is not to be afraid to try something different, something new, right? Even though something you didn’t have a background. So take the stab and probably look at you now, right? It’s partly your main life now.

[00:09:05] Looks Good to Me (LGTM)

Henry Suryawirawan: So today we are gonna cover this book titled Looks Good to Me, which you’re still writing, right? It’s a book about code review. First of all, maybe a bit of background, what is Looks Good to Me, or in some company, they call it LGTM, and you are writing this book.

Adrienne Tacke: Great question! So LGTM, the infamous acronym, or, uh, some people use the thumbs up emoji, right, with that. This is usually used to say everything looks, you know, okay, it’s good enough. It passes muster, it’ll be just fine. And this notion has kind of devolved, I think, into something that’s not so good. As a software developer who goes through code review and reviews other people’s code or, you know, puts up their own code changes to be reviewed, there’s a lot of things that have just become really, really, . mundane or unmanageable about the process. And that’s really sad to me because this process is a very key part of software development and our jobs as software developers.

So code review for anyone that might be unfamiliar is this very, I’ll start with a very basic workflow of you have code changes that you create. Usually we call this the author of the code changes. Once they’re ready to have their code reviewed, they typically put it up in what’s called a pull request or a merge request or a change request. And they prepare it for someone else, a reviewer to look at. And then this review process can take many forms. Sometimes, it’s tool-based. A lot of people are familiar with the GitHub based thing where you put up a pull request and it’s asynchronous. Someone looks at it online on a tool. And there’s a code diff that shows the differences between the changes and what has been done. Sometimes, uh, people like to consider pair programming sessions also a code review, where you may write some code and someone looks over your shoulder and reviews it at that time.

Now review, this phase can mean many different things. And this is kind of what I go into in my book too. But, you know, you have different goals as a team when you do the review part. Sometimes it’s looking to make sure code looks correct. Sometimes it’s seeing does this match the rest of our code base? Will this improve the health of our code base? Is there anything outstanding that’s missing? And are there things that we can learn from the changes? Like why are we making these changes at this time? And kind of getting the rest of the team on board as to what’s changing in the code base.

And once that’s done, once a couple people, ideally other than the author, the one who actually wrote the changes take a look at it, that’s when you get that infamous looks good to me. You know, thumbs up, an approval that says, yay, this is okay. And then it goes on and merges into the rest of the code base. So, very simplified at this point what I’ve just explained, but that’s kind of the basis of what a code review is, and you’d be surprised at how many things can go wrong in that process.

Henry Suryawirawan: Right. I’m sure all developers understand what you’re saying, right, about getting it wrong, about getting it mundane, right. So first of all, I think I just want to clarify, you mentioned a few things that, which I think has been associated with code review workflow for quite some time, right?

[00:12:24] Code Review Story

Henry Suryawirawan: The first is about PR, MR, or CR, right? When you said it can also be done through pair programming, right, it doesn’t mean that review can only go through, you know, pull request, merge request, change request kind of mechanism. And the other thing is, I think about the process of merging. So we are assuming people are using version control, of course. And a different kind of branch before they can merge. Any kind of maybe fun stories or horror stories that you experienced with code reviews that you can share with us?

Adrienne Tacke: Yes. So my very first experience was actually with a Team Foundation Server. I’m dating myself a bit, but this was Visual Studios and Azure, but Microsoft’s way of version control. And this was still when it was kind of new. People were not really big on version control yet, but it was the system where you would kind of check out a file and you could work on it. And the notion was that kind of like checking out books in the library, when you check it out, you have your changes, you work on them and then you’re able to check it back in. And this checking in process, at least in the team that I worked on, we didn’t really have a review process. You kind of just assumed everyone that was checking out their things were reviewing it themselves and kind of gave them the accountability that, hey, anything you are checking in is on you, right. We hope that anything that you check back into the main project code base is good and that there’s nothing wrong.

So, that was my first experience dealing with, well, what happens when somebody checks it out at the same time? Or there’s a split difference where they may have overwritten something that was just checked in. Or you know, people don’t have the accountability and they just kind of check in their own changes and you don’t know that it’s bad. This is kind of the beginnings, the foundations of where I’m like, why isn’t there some sort of process where we kind of take a look or just hold on for a second before we are sure, let’s take a look at what’s about to go in?

And I knew at that time, I was very afraid to break things. So I was extra careful. I was like double checking, triple checking. And it was really hard to learn that not a lot of people are like that. Some people are just like, yep, let me just put it in and if it breaks, it breaks. And I didn’t have that mentality, so it was very difficult to experience that firsthand of what happens when, in that sense, it was kind of like a merge conflict where the same file was changed and you don’t know what’s proper, you don’t know what to keep, you don’t know what’s most current. And yeah, that, kind of started out the basic foundations in my career. I didn’t know it yet at the time. And as I started to learn Git, those things crept up in my head. But yeah, that’s where it kind of all started for me and got my mind spinning about why there was no such review process on my own teams.

Henry Suryawirawan: I think I must also say from the experience I had, right, the kind of version control that you use will also determine this kind of a code review process, right? So I think mostly people are familiar with Git now, which is kind of like optimized for this kind of workflow. So I think we should appreciate that Git exists, right?

[00:15:38] Importance of Code Review

Henry Suryawirawan: So in your book you come up with this statement, which I think really, really interesting for us to discuss, right? You mentioned that we need actually code review, but most of us actually dread it. Tell us why many people actually dread code review process.

Adrienne Tacke: There are a lot of reasons over my career, over the different people I’ve spoken to, different developers. It mainly comes down to a couple things. One, the process is unsustainable. So what I mean by that is the process itself is just really cumbersome. There’s too many steps or the PRs, the reviews themselves. There’s too many files to look at. There’s too many lines of code to review that’s way too large for any single person to properly look at and give a review. Sometimes there’s no description or context that comes with the code changes.

So as a reviewer, it’s very difficult to make sense of what it is that you’re trying to do. And then there are just processes where if you try to leave feedback, you don’t know how to leave feedback. Or the feedback is not helpful, something like, “oh, this could be better”, and then that’s it. That’s the only comment you get. And you’re like, okay, what am I supposed to do with that? As an author receiving that comment, you have nowhere to go from there. So, it’s become unmanageable, unsustainable.

Number two, in the more extreme cases, there are reviewers who kind of take advantage of the little power that they do have as a reviewer. And sometimes they just don’t know how to write effective comments. So there are a couple examples in my book where I share, these are on, you know, public open source repos. And people are very harsh in their comments. They’ll say, “Oh, this is a stupid way to implement this.” Or “oh, this looks terrible.” Or, you know, this or that. And I always think for someone who’s either new to software development or new to the team, or someone who’s just unfamiliar with the actual technology but could be a seasoned developer, when you have this kind of culture of just being so harsh and not being able to give constructive feedback, that definitely does not cultivate a good developer experience.

You know, there’s another thing I say, which is a lot of developers tend to tie their self-worth to their code, which we shouldn’t do. I talk about that too, but, you know, sometimes you do. And for those who haven’t learned to uncouple themselves from their code, getting a comment that’s that harsh or direct or just unnecessarily mean, can really break their day, can make it really unnecessary. So a code review that’s unnecessarily mean is something people dread.

And then the last part is just, when you go through the process, sometimes it’s so stringent. So there’s like the lack of code review, which is a terrible thing, right? You get this wild, wild west of everyone stepping on each other’s toes. That’s not so prevalent now. Like you said, the Git is more common and not to say we all use it properly, but it’s there. And then there is the other side of the spectrum where it’s too stringent. So now you’re dreading the process, because you have to get like this approval and you have to fill out these things and you have to do this and that and that.

And you know, in my book, I do say we’re not supposed to think about it as a burden. There are certain things we have to do, like if we’re opening a change, we need to make sure context is there. We need to link it, we need to put labels, but that’s a bare minimum thing. I’m talking about the processes where they have extra approvals. They have so many unnecessary steps that… what do developers do when they, you know, are inconvenienced at any step? They’re gonna go around it. So any process that is made much more difficult than it needs to be without any additional value, developers are gonna find a way around that. And so that’s what I’ve seen are a few ways why developers just dread the process. It’s not as easy as it needs to be. It can be very harsh and mean when it doesn’t have to be. And it is unmanageable when it should be manageable, and it should really reel in all of the benefits if it’s done properly.

Henry Suryawirawan: I’m sure like I can relate to many of them. I think it also boils down, in your book, you mentioned it’s a lack of communication and trust between team members, eventually. You know, like these mean comments or ineffective review comments. Or maybe even the delays and number of lines of code and number of files that you submitted, right? And also I think the lack of clarity about the process itself. I think we assume that everyone is expert in doing code review, which I think in my experience, not everyone actually is an expert and we need some kind of resources like this book to actually upskill ourself.

[00:20:27] Code Review Benefits

Henry Suryawirawan: So, apart from the obvious, you know, like looking at the code, finding bugs, making it better, what are some of the other benefits that you can share for doing code review?

Adrienne Tacke: Yeah. So even finding bugs, that’s a highly debated thing that people say happens. I talk about that in the book. There are a couple studies that do show, like there’s a Microsoft study that actually says it doesn’t find bugs. But, uh, depending on how your team uses it, sometimes it does. We’ll leave that up to debate.

One of the really good things that code review does is it leaves artifacts for your team. So it leaves a historical record of what changes in your code. And it also increases knowledge transfer among the team. I think, to me, as I’m continuing to write this book, those are the really the key things of why code review really will stay. We might talk about this later, but people are talking about, oh, AI can just do the code review for us and we don’t have to deal with it anymore. That’s not gonna happen.

But knowledge transfer is a really big one. You know, as you code review and more eyes look at the code changes that are being presented, that’s one of the greatest ways to share what is happening on the code base. And again, if the code changes are prepared properly, as in you give the context as to why the code is changing, you tie it to the tickets that may have started the, you know, the reason for why you’re making the change in the first place. Or, you know, you tie it to the initial bug report that ties to the fix you’re about to change, it just paints a better picture for the rest of your team as to why you’re making this change. And then the diff itself, the code, that’s the how, right? How are you doing it? You’re looking at the code, you’re seeing what you are implementing. The code review, the PR, the pull requests or the change requests. That’s the why. And so that is very important to spread across the team.

And as you go and evolve your code base, it’s very important for the team to all be on the same page. Otherwise, you get these scenarios where it’s a single person or it’s like a senior senior developer, and they’re the only one that knows anything about everything, about the code base. So then you get this problem where they actually become the only person who reviews every single pull request. And then you get into the unmanageable case where it’s like, uh, it’s a bottleneck. I’m the only one who can look at these. Well, why are they the only one that can look at this? I’m the only one that knows about this code. Why are you the only one that knows about this code? Well, because I’m the only one that’s familiar. So that in and of itself is a great reason to do code reviews and to do it with as many people on the team as possible to learn about it.

The other side of that other really good goal of code reviews is historical record, specifically, traceability. So when you have these official formalized documented reasons for why the code has changed, it makes it much easier for the team to see what has happened and if something goes wrong – If you do it correctly, or I wanna say, if you take the effort and time to, you know, have really good commit messages, to link work tickets to the pull requests, to make pull requests and changes atomic and small, and to do them in a way that if something does go wrong, you can easily revert back and say, oh, I know when that happened, it specifically happened October 31st on this specific commit. That’s the goal, right? That’s the ideal state that developers would love to have. But that’s possible with code review because you get to see what has happened, why it has changed and you are where you are today.

And then if you use, you know, there are things like, I just watched a talk at a conference I spoke at where they use Git release manager and they don’t use pull requests, but they do use issues as kind of their central item that kicks off everything. And because they have good commit messages, because they use proper linking with issues and with pull requests and labels and things like that, they are able to automatically generate a change set. And then also if there was an issue that was fixed by a specific pull request, they automatically are able to send a message through a webhook that says, hey, we just released something. This has been fixed in release, you know, 2.3 in this particular thing.

So a lot of that accountability and traceability, you can get if you do these processes. And I think those are really, really good things to have. That’s what makes software development much more professional and much more fun, I think, if you have it all laid out, uh, in a nice way like that.

Henry Suryawirawan: It seems like a really high bar and really interesting to learn, as a case study, right? So I think things like spreading the knowledge, we kind of like think it can, it will happen, right, in the code review, but not all the time, I believe. Because like what you said, right? LGTM is most common comment probably when you’re looking at the PR, whether it happens constructively or not, we don’t know. And many other things, like for example, making sure the code base follows the same standard in the team, right? I think that’s also one of the other benefits, if done properly.

[00:25:50] Code Review Role: Author

Henry Suryawirawan: So I wanna tackle the other topic that you’ve covered really, really well in the book, which is about the roles and responsibilities of the people or entities involved in the code review, right? So you mentioned like reviewer, author, and things like that. There are actually more roles in the code review process. Maybe if you can go highlight some of the roles and responsibilities, especially the author. Maybe let’s start with the author first.

Adrienne Tacke: Sure. So the author, like I mentioned, this is the one that puts up the code changes. They are the ones that write the code and they’re asking someone to review it. And for the author, I put a lot of responsibilities. I kind of call it a contract for them to adhere to in the code review. And one of the first things is to kind of forget your ego. So we touched on this earlier. Like I said, sometimes we feel like our code is an extension of ourselves. We’ve been working on this really nice change or we’ve fixed this bug and we’ve prepared this really nice code and it’s our baby, right? And so when we put it up for review, we become vulnerable now. We don’t wanna hear sometimes feedback on it because, what do you mean, it’s perfect the way that it is? Our baby is perfect the way that it is.

But sometimes not being open to that feedback means we don’t get the best code possible. Or when we are so in the weeds working on our particular feature, just being down at that level, focusing on the code, sometimes we don’t see particular edge cases or sometimes different skill sets and different perspectives take a look at our code and can actually offer some good feedback. So being open to that means we have to let go of the ego of, you know, our code is perfect and the way that I’ve submitted it is perfect.

Alongside of that, you might be open. You know, you’re like, no, I know that I have room for improvement. Sometimes people will get feedback, but then they’ll say, oh, well actually no, I don’t think that is correct. So that’s the other side is, you know, you say you’re open to feedback, but then you actually do get feedback, and then you’re like, no, you know, you disagree, and you try to fight it off. So being able to let go of your ego and understanding that if we’re asking the reviewers to objectively review our code and to only look at the code, not consider us, then we need to do the same thing. We need to objectively say, this feedback is based on the code as I have presented it. It has nothing to do with me, has nothing to do with anything about me. It’s just the code. Focus on the code.

The next thing that authors are responsible for is we need to have due diligence when we actually create these pull requests. So I don’t know about you, Henry, but have you ever received a pull request where it’s like the title is like bug fix and then everything else is empty and you have no idea what is going on. Have you ever had like a very nondescript review that you’ve had to do?

Henry Suryawirawan: Yes. Not only PR, sometimes the code commit comment as well.

Adrienne Tacke: Yeah, uh-huh

Henry Suryawirawan: Yeah.

Adrienne Tacke: So that is another big thing that I see authors don’t do, is we don’t set up the reviewer for success. We don’t give them the context that they need to properly review our code. So that starts with a very good descriptive title. Like, I need to know what I’m about to review in the 10 seconds that I read the title. And then when I go into the description, the description is where you tell me the why. Why am I making this change? Is it because a ticket was the catalyst for this? Is it because we had a planned, you know, in our retrospective we said this work was going to be done and this is now the work that is happening. Is it because it was some one-off conversation with our manager that said, you need to do this right now. Like, why is it here? And then adding that context there. It tremendously helps put the reviewer in the correct mindset to review it.

Not only that, but making our PRs manageable. So, I’ve been guilty of this before, especially when I had, you know, I was just learning Git. But you know, there are. PRs out there with like 5,000 files changed or 8,372 lines changed and you’re like, really? You really expect me to properly review this? A single person So not making the PR manageable is something I think is a very big responsibility that we have as an author. So that’s why in my book I do have, you know, rough guidelines from several research papers and studies that have been done.

Basically, the smaller that you can make your PR, the better. And it makes sense for everybody involved. The more atomic the change, the less files you have in your PR, the less lines of code that someone has to look at. The more likely that the person, the reviewer is going to be able to properly review that. The less likely someone can have an excuse to say, oh, I don’t have time to review. Because it’s such a small PR. If you can do it in 10 minutes, it’s gonna be reviewed. There’s no excuse to not review it.

And then again, it’s the traceability aspect of if something goes wrong, it’s much easier to pinpoint this particular pull request was the source or the root, right, versus having like a gigantic pull request with several changes muddled in there along with configuration changes and some package JSON changes. Like you put that cognitive load on the reviewer and on yourself later on if you have to ever have to untangle something from that particular pull request. So that’s another responsibility that I say is very important for authors is to make sure that the pull request that they’re preparing is primed, is what I say. It’s ready for the reviewer to look at. And I think those are the big ones. I’m sure there’s a couple more that I’m forgetting, but those are the ones, off the top of my head, are very, very important that we tend to forget.

Henry Suryawirawan: Yeah. You mentioned about cognitive load. I think sometimes we don’t empathize the reviewer, right? So we are in the mood to finish whatever the things that we are working on, right? And we didn’t realize that the number of files, number of lines of code are too big that we just submit to PR in one go, right? So I think you mentioned a very, very important responsibility as the author as well is be your first reviewer, right? So you look at the PR, at number of changes and make it more manageable, right? So I think making it manageable to me is maybe one of the biggest important thing that we can do in order to make it less dreadful.

[00:32:42] Code Review Role: Reviewer

Henry Suryawirawan: So now the author has created the PR, so there’s a reviewer. I this is also another important role. So tell us the responsibility for the reviewer.

Adrienne Tacke: Yes. So for the reviewer, I also say the same thing, right? Forget your ego. Focus on the code, not the developer. So as reviewers, we need to be objective. We can’t bring in, you know, if you’re having like an argument with your coworker, but they ask you to review your code, that’s like another issue. You should sort that out. But if you are reviewing someone’s code, that shouldn’t come into play. You should just look at the code. This is also not the time to be giving a better implementation for the sake of giving a better implementation without reason, right?

So, a lot of what I hear when I talk about these reviewer responsibilities is that, this means that I can’t just give feedback anymore at all to the author. Like it has to be walking on eggshells, everything is happy. I’m like, no, that’s not what I’m trying to say. What I’m trying to say is, if you have feedback to give, you need to do it effectively. So that’s another thing that I say is a responsibility of the reviewer is to actually learn how to write effective comments.

This is something I think a lot of reviewers have trouble with. And rightfully so, some are actually empathetic, right? They’re like, I don’t want to offend the developer. I don’t want to hurt their feelings. I want to bring up something that is important, but I don’t wanna do it in a way that will scare them off or make them upset. And so something that I’ve developed in the book, I call it the Triple R pattern. If you are gonna ask someone to make a change or you have a request or something, I say the three R’s. Name the Request, give your Rationale for it, and then give a Result. And that will almost always be an objective way to write a comment that gives feedback to the author. Because you tell them what you want, which is the request, you give the reasoning behind it, which is the rationale. And then the result that you are sharing with them is a way for them to actually compare and say, am I on the right track? Or did I do it correctly?

And that could be, you know, if you need something to change visually, have a screenshot exactly of what you are expecting. Because that will almost always be helpful than trying to describe it. If it’s like a performance optimization, give them a benchmark to hit. The more specific that you can be, the less likely the author is gonna take this as feedback that says, oh, this person just wants me to do more work. Or, you know, they’re trying to make me achieve a standard that’s unnecessary right now, I just need to get the work done. The less you are vague about it, the more outcome focused you are, the more specific you are and the more objective you are about it, the better your comments will be received by the author.

With that, another thing that I say is a responsibility of the reviewer is we should also be thorough in our reviews. So, there’s been a notion to kind of skim reviews. This is why we get the looks good to me, right? And as an author this time, when you know you’ve prepared a nice review, maybe you have a nice set of changes, it’s actually manageable, and you put it up for requests. You’re like, oh, can someone review this? And then within like two minutes, you get a thumbs up. And you’re like, there’s no way somebody looked at this properly. There’s no way they actually gave this the due diligence that it needed. So as a reviewer, you know, it can be tempting, but this is why it’s a two way effort of when the PRs are manageable and there’s small enough, then there’s really no excuse for you as a reviewer to skim them. So, if they are small enough and they are manageable, being able to review in 10, 20 minutes at most, give them the proper review that it needs, go through them, go through the code, and really give it your thorough eye as if you would want to have your own code reviewed.

And then also giving that time to yourself. Sometimes you need to talk to yourself and say, you know, before you ask the author to make a change, for example, first ask yourself, why am I asking the person to make this change? Because if you yourself can articulate why you are gonna make that request, then you’re gonna be able to better articulate it to the author. That’s how you get rid of those comments of like, this could be better, and then it’s done. Or, you know, a comment that’s like, I think there’s something wrong here, but then that’s it, and you leave it at that. That’s where the discussions go back and forth, because you didn’t take the little extra time to articulate it a little bit better or to understand yourself why you’re making that comment. And so, you know, doing that due diligence, not skimming in your review, giving it a proper review, and, you know, forgetting your ego and making sure that we write effective comments right now are, on the top of my head, the most important responsibilities for the reviewer in a code review.

Henry Suryawirawan: Yep. Also you mentioned something really, really important I want to highlight here as well, right? Eventually, the reviewer also responsible for the code that gets through right to the production.

Adrienne Tacke: Yes.

Henry Suryawirawan: So I think if we just skim it and we think that it’s the author’s problem, I think that’s also not the right way, right? So I think the reviewer should also own the responsibility to make sure it works really, really well.

Adrienne Tacke: Yes. I’m glad you brought that up. I’m really glad you brought that up. The last thing I’ll say about that is, there’s this notion with reviewers to be like, oh, it’s the developer’s fault, right? If something does go wrong and it goes through their review, we are quick to blame and say, oh, it’s not. And really what we should be thinking is, oh, how did that actually pass through my review? So that’s like a really big mindset shift that I’m hoping to convey in the book. And I hope more people will start to think about when it comes to code reviews, specifically as a reviewer. But thank you for reminding me of that part.

[00:38:41] Code Review Role: The Team

Henry Suryawirawan: Yep. The other aspect about reviewer is like how long the code review lasts, right? So I think there’s sometimes a power dynamics, especially if you are senior to review your code, right? You kind of like, just wait and kind of like beg them, please review my code. So I think as a reviewer, you also have the responsibility to make sure it doesn’t drag, right? It doesn’t delay for too long. Which brings us to the aspect of team, right? A team, maybe standard or convention, right? So the team here also has a role to play. So tell us, what is the responsibility for the team?

Adrienne Tacke: So the team is basically the one that sets the tempo for the rest of, you know, for all the individuals. Sometimes there are some suggestions in my book, where I say have extra steps, you know, oh, use a lot more caution and be very intentional about what you do. But sometimes you have a team who might be like a team of five senior developers. They’ve all been working very well with each other. They know each other. Maybe they have the luxury of actually being in an office on-site together. And so if that’s the case, maybe you may not need such a formalized process. Maybe you trust each other and you have less of a checklist when it comes to your code review. But then if you have maybe say a more mixed team, you have a couple senior developers, you have a more intermediate and junior developers, or you have an intern or someone that’s maybe new to software development, then it’s the team’s job to help integrate the rest of the members so that, number one, they know what the process is. And number two, to keep the process enforced for everyone.

Doing that becomes a lot easier. There’s a whole chapter I dedicate to this concept, which is what I call the team working agreement. And this is a document that’s co-produced by the team. It’s basically all of the implicit expectations that you have as a team. You make them explicit, you write them out. So what are the steps of your code review? What are the expectations around the code review? Things like, if somebody leaves feedback on my code review, do I have to answer every comment? Do I have to mark all of it as resolved, or is there some wiggle room there? What counts as done? What counts as an approval? How long is too long on a code review? If someone hasn’t looked at my code, when is it appropriate to follow up? These are all the sources of frustration, of tension among the team, because it may be different for everybody in everyone’s head and specific preferences. You know, someone might want it done the next day, 24 hours. Someone might be more lax and like, oh, I’ll get to it by the end of the week.

So having all of that figured out on your team not only makes it easier for everyone to be on the same page, but if someone is going off base and is not following the process, then you can use that document and use it to enforce on the team to say, hey, look, we all agreed this is what we said for this particular situation, and this is what we should be following. And so it’s much easier to bring that up versus just saying it’s a he said, she said, or you know, it’s one preference versus somebody else’s preference. It’s the team’s preference and this is the team’s standards and this is what the team’s way of going about things.

And the other thing about this document is that, you know, a lot of people say, well, you know, once these are written, like, what if things change? Well, the document changes with the team. So it’s not this set in stone thing where it’s like, these are the rules forever. No, your team’s gonna change, your code base is gonna change, your project’s expectations are going to change. There are a lot of things that are gonna change. And so if you find that there are parts of the process that don’t work for your team, change it and then also change that team working agreement.

That’s the great thing about this is that as long as you have an open culture on your team, a culture of feedback where people are open to talking to each other and open to discussing things like this, then a change can be made the next day. And then, you know, it might take a while for people to get used to it. But the change is implemented and then you continue to enforce it that way. So that is one really good way to keep the team accountable.

And then the last part is not everything is gonna go the happy path, right? It’s not just we’re gonna change some code, we’re gonna review it. Some people look at it and then it goes into the rest of the code base. There’s gonna be times where there are emergencies. So production goes down. There’s some really big thing that happens that requires the attention of the team. What do you do in that situation? This is where a lot of other problems become larger when you don’t have a process in place. So sometimes emergencies are used to circumvent the code review process, and that’s not a good use case for it.

What would be better is to anticipate emergency scenarios and then have that written on your team. Say, what do we do in an emergency scenario? There’s things called an emergency playbook where you write down, okay, if it’s this kind of emergency, what are we allowing in our process? Are we saying we can allow one approval instead of two? Does a manager approving, you know, does that count in an emergency situation versus our normal two people or more? Could we open a pull request straight to production? What are those processes and what’s happening in them? And then not only that, if you are gonna go around the process, your normal code review process, you make sure that in your emergency processes you have a very outlined detailed communication plan of this is happening and this is why we’re doing it.

So it’s one thing to circumvent the normal process, it’s another to make sure everyone knows that that is happening, knows that’s okay because you’ve anticipated it and also knows that you’re circumventing it because it’s an emergency. So that’s very important to anticipate it and then to communicate to as many people as possible that whatever you’re doing is happening because of the emergency situation.

Henry Suryawirawan: Right. I find these two, the team working agreement and also emergency playbook is something that everyone should check from your book, right? Because it really details a very, very important thing that I think most teams are lacking, right? Especially the team working agreement. So what you describe is like you make something like an expectation becoming more explicit and that you can enforce to the team, right, as a standard practice for them. So do check these two out from the book. And I think really, really, a lot of insights there. Of course there are other roles which we will not cover. Things like who is in charge, right? The TL, the EM, or even the leaders from engineering, right? And also the organization at the end.

[00:45:53] Code Review Tools & Automation

Henry Suryawirawan: So let’s move to the next section which is about automation, right? I think in some teams which are advanced enough or using some system that provides all this automation is always nice. Tell us the role of automation in making code review better.

Adrienne Tacke: So you’d be surprised at how many teams that I’ve spoken to that don’t make more use of automation in terms of development automation. So there are a lot of code reviews that are spent fixing formatting or fixing spacing or changing naming conventions to a particular language. And these are all such low stakes issues that they really should not be in the code review, because they waste a lot of time and they are also very subjective. They can get into very heated debates among developer preferences. And so that’s another thing that should be decided upon by the team. Have a proper style guide, add that to the team working agreement, and then make use of the automation that is for that.

So linting, formatting, static analysis, running security tests that you can locally, these are all things that should be done during development. We have so many tools at our disposal as developers that integrate with our IDE. We have extensions galore in VS Code. We have, you know, things like ReSharper. There are so many things that are there to help us to do all of these things during development.

And I really believe that if you fix all of these things as they happen, as you write code and when you can during development, the less likely they’ll interrupt the code review process, and the more likely you’ll get to use the code review process for those more important things, for actually focusing on the things that matter, either allowing the reviewer to actually just see higher stakes issues, because they’re not bogged down with like, oh, there’s a space here, or you’re missing a comma here. You are letting them focus on higher stakes issues, but you’re also allowing people to actually get that knowledge transfer without being interrupted or bothered by all of these lower stakes issues. So automation is for those things firsthand so that you can get rid of all of that noise in the code review.

And then another part of automation that I know sometimes it’s not really part of the code review, but I’m big proponent of, because sometimes you can integrate it with them, and those are all the kinds of checks that you can do in a process. So on GitHub specifically, and on some other tools like Azure repos or I’m sure AWS has an equivalent, there are some initial checks that you can do. For example, on GitHub, when you open up a pull request, you can set it up to have an initial build go through and see if it will break, and it’s a pre-built check. There are other tasks that you can have run also when you open a PR. So if you have things to check, like, there are some GitHub Actions, I think, that allow you to check for a multitude of things. Do you have secrets in your code? Do you have some security, you know, like OWASP 10 issues that are glaring that somebody else may not have the time to check? Um, those should certainly be checked during development, but they act again as another checkpoint at the PR point to be able to check that as well.

You have things like being able to automatically deliver, let’s say, a PR template to someone depending on the type of issue that they’re doing. So, you know, I always go on and on about how the author needs to put as much information as possible in their PR. Sometimes that’s a lot of information. Sometimes you just forget or you really don’t remember all the things. And so something like a PR template, for example, you can have a checklist basically that says, hey, you help the author. You say, these are all the things that you need based on the type of pull requests that you’re opening. And then again, you set them up for success. You know, you let them help themselves by having this automatically appear when they open a PR.

So using automation in this way is, again, it’s to help us help ourselves, right? It’s to get rid of all of the noise, all of the lower stakes issues, and to give the reviewers the best chance of reviewing it in like a clean of a slate as possible. To really focus on the things that the automation can’t find. That’s where the human judgment, the nuance, the understanding. The example I really like giving is you can have all of these checks, you can run all of the static analysis and all of that, but the reviewer is the only one that can, in a code review, say, hey, this variable name is actually not that meaningful. I don’t understand it. Can you make it more meaningful? And meaningful is completely lost at least at this point for AI, for any kind of static analysis tool. It would never be able to give that kind of surface, that kind of comment to help us fix it. So if we can eliminate all of the noise and allow the reviewers to focus on those kinds of issues, then I think we’re doing a good job and we’re really using the code review to its largest advantage and largest benefit.

Henry Suryawirawan: I think, if as a team, right, you manage to, you know, remove all these accessories thing, I’d say, like formatting, spacing, you know, comma here and there, right? I think that’s a good bar that everyone should aspire.

[00:51:44] Effective Code Review Comments

Henry Suryawirawan: Another thing that you include in this automation is actually inclusive language kind of a check, which brings me to the next thing about commenting, right? Maybe a little bit of advice how we can comment code reviews better.

Adrienne Tacke: Yes. So, aside from the really good pattern I shared, which is the Triple R pattern, the, you know, Request-Rationale-Result, I think, following that for most comments would be very good because it gives the author somewhere to work off of, and it also gives them some understanding or context behind why you’re making it a request or that comment.

But other things, I don’t know if you’ve ever used a nitpick comment label. This is very debated. Like I feel like you should not have nitpicks at all. But if you must have some sort of nitpick, which is like, it’s purely subjective, but you have to tell the author that this is, you know, you don’t have to do anything about it. It’s not a change. You just wanna let them know this is purely a subjective, preferential thing. Then, my team likes to label that as a nitpick very clearly. And along the same lines, if you’re able to kind of label your comments in that way, for example, if something needs to actually be done, make that very clear on the comment. So just like you would label a nitpick “nitpick” at like bold first nitpick, and then the nitpick, you would say “needs change”, for example, and then you write your comment. Or “needs rework, let’s take this discussion offline”, and then you put your comment.

You know, having that very clear upfront and being able to let the author read it, having them read that as the first part of the comment saves a lot of cognitive load for them, because then they can say, oh, these are the ones that are important that I should focus on. And then these are the ones that are not so important. And I think that is important, because I don’t know how many people are like me, but every time I see any comment on a pull request that I open, I just immediately associate that, oh, it’s work I have to do. Every comment means I have to change something or update a line or do something. So then I get a little bit worried, I’m like, oh, I gotta do all of these changes. But then being able to organize my expectations as an author and says, oh, I actually only have one thing to work on, the rest of these are nitpicks. That’s a much better way to receive feedback as an author. So that’s one thing to help in the discussion.

And then, there are also code compliments, which I’m a big fan of, but within reason. So like, you know, everybody thinks about code review as this very judgmental process. You’re supposed to find what’s wrong with each other’s code, and it’s part of why people dread it, right? It’s typically seen as a negative thing. But if we try to turn this around and we say, you know, let’s say somebody implemented something in a really novel way, like you haven’t seen it before, or they have a really awesome performance savings that you just are very impressed by. Say something, you know, say that’s really awesome or good job in figuring that out. Or some like bug that everyone has been stuck on and then someone actually finds a fix for it with tests – we hope, right, and then documentation. And all of these things, please feel free to actually leave a comment or two that’s a compliment. If you see something you like, say so.

Where it gets too much is if you’re leaving it for the sake of leaving it, you know, that’s where I draw the line. Like if every other comment you’re like, oh, this is awesome or, oh, that’s great or, oh, this is, it kind of loses its value because now you’re saying everything is great, everything is awesome. And so I say code compliments within reason. If you find something really novel, I’m sure the author would really appreciate and like that you found some appreciation in the way that they wrote something.

Just in general, like you can sum it up as don’t be a jerk. I know that might be harder for some people or maybe some people don’t know if they are. Like, you know, some people are more direct, some people are not as direct. So maybe that’s actually one way where I would say AI, an AI-based tool could help where if you are unsure, you can, you know, use an extension that says, oh, your sentiment is a bit more negative, or it seems passive aggressive, or, you know, the tone could be more neutral.

Something like that is where I think AI-based tools could help in a code review. But there are people that say, you know, let’s have AI write the comment for us. Sure, you can, but again, I always hesitate to say fully lean on that, because you still have to review it. Anything that is AI-generated or AI-produced, it still needs to be reviewed by a human. So, sure if it saves you some time and you can let some tool write the comment for you in a more neutral tone, just know that you still need to look at it to make sure it’s correct. And number two, to make sure it’s actually saying what it wants, you know, the intent of what you actually want it to say, to make sure that’s still in there. So you don’t wanna lose that part of the communication in there.

So in terms of comments, yeah, you know, code compliments within reason. Don’t be a jerk, and try to be as objective as possible, as outcome focused as possible. And just try to write the comments that you would want to receive as a reviewer in the most objective way as possible.

Henry Suryawirawan: Yeah. Don’t be a jerk, I think have a lot of empathy, right, for the people who either the author or the reviewer, right? And also . I think it’s very important, like you mentioned, forget the ego, right? Like you should not have ego. Attack the code, not the person, right? I think that’s also important.

[00:57:45] Code Review Turnaround

Henry Suryawirawan: So one very crucial aspect, the last question I have from the conversation is about, you know, code reviews that takes too long. I think this is quite common in many teams. This is also an anti-pattern, I believe. What can you say for teams to actually maybe making the code review speed faster. And also interestingly, right, the last State of DevOps report mentioned about code review speed as the one technical practice that teams should aspire to become a more elite kind of a performance. So maybe a little bit advice here.

Adrienne Tacke: Sure. So code reviews can take a long time due to a lot of reasons, but it mostly comes down to the PR itself is too large. And then the PR itself being too large is a symptom of other issues. So we’ve touched on the other ones before, right? They take too long because they’re gigantic. They take too long because the author has not put in enough information in the pull request. So as a reviewer, you’re kind of sifting through, you’re trying to make sense of what’s happening, and you can’t really understand why this code change is happening the way it is.

If you have a person, a single person who’s doing a code review, that could be a reason why it takes too long. So it’s something I call the single senior developer bottleneck problem. They’re the only person that can review, which means they must be on all the PRs and they’re only a single person. So, you know, take that into account and then multiply that. If the PRs that they have to look at are large and are non-descriptive, you get the idea. It can take very long that way. So we’ve talked about how spreading the knowledge might ease that problem. You level up the rest of your team so that there’s not just one reviewer available, but multiple reviewers available who are able to share the load.

And then the thing that I said, which is try to make your PRs smaller. So one thing that is very common is, you know, I say atomic changes or like keep it to a single feature or single bug fix. And sometimes the feature itself, it is a single feature, but it’s still like 30, 40, 50 files or a lot of lines. And so when it gets to that point, the thing you have to look at at that point is maybe there’s something that’s lacking in your design or planning phase. And so why did the feature get scoped to be that large? Did it actually start out small and then scope creep made it that large? Or did you actually all agree and say, no, yeah, this is a single feature, I think we can do it and this is gonna be fine.

If it happens to be that case, that’s where you need to start planning on your next design and planning phase and for the foreseeable future. If you don’t want large PRs, then you’re gonna have to scope your features down much smaller. So really think about the feature and if it has to have everything that you are thinking about, because if you absolutely must have it all together in a single feature, or at least in one task that is being worked on, then just anticipate that the PR is going to be that large, especially if you’re trying to keep to the single feature PR role. So, just to have people be a little bit pedantic about that. It’s more about is the change itself as small as it can be. And then if you are able to get it that way, the resulting PR is going to obviously be much smaller.

So the other thing that makes PRs really, really long is, let’s say, somebody asks a question, somebody answers back. Somebody doesn’t understand, so they ask to clarify. And then someone tries to explain it and then they don’t get it, and then it just becomes a really long discussion online asynchronously. If you have the ability to get on a video call or if you have the luxury of seeing the person in-person, sometimes it’s best to just, if it’s like two or three comments or four comments in, just take it off and actually speak with each other synchronously. Because then you have a greater chance of getting through whatever communication block that is happening. You can walk through the code together, you can go through the description and have them fill in, what is happening.

Sometimes the result of that conversation means there’s actually stuff that was missing from the description. And so you can just ask the person, hey, can you add this to the description? And then everything’s fine. Sometimes that discussion actually reveals that, oh, this is actually work or changes that don’t need to happen right now and we shouldn’t have this PR. And then you find the reasons for it, and then you close the PR because you don’t actually need it.

And then the important thing here is to actually let everyone else know on the team, hey, we are closing this PR because of X, Y, Z reason that we figured out on this discussion. This is the important part about discussions that happen offline is whatever the result of that discussion is, it’s important to let everybody else know. Not just for knowledge transfer and for any of those teammates who may be looking at the PR with you, but later down the road, again, the documentation, the historical aspect. If you are wondering why did this PR close or how come we never merged this in, this actually would’ve fixed something. And then if you just leave one single comment that says, hey, we took this conversation offline. We talked about this, this, and this, and this was the result of our conversation, this is why we’re closing it. Think of how much headache you would’ve, you know, eliminated in the future, even for yourselves, because you just took the time to write that out, right? So if you find discussions are getting too long, try that. Try taking it synchronously and trying to work it out. And then please go back to the PR and write down what happened as a result of that discussion.

So all of these reasons, they tend to prolong the code review process, because there’s just too much cognitive load for the people involved, right? And that can come from the multitude of reasons that we’ve spoken about. But if you focus on making the PRs smaller, if you focus on, you know, erring on the side of communication and trying to talk to people and getting that understanding faster, I think a lot of the reasons that make code reviews long will actually go away. And then code reviews will be the quicker, more efficient process that they’re supposed to be.

Henry Suryawirawan: Right. And I would also say that make it as part of the work, right? Make it as part the definition of done so that you kinda like expect code review to be done as part of the work. And I think what you mentioned, all of that could be included in the team working agreement. So again, I think it’s a very, very important artifact for the team to kind of like agree on the expectations explicitly.

[01:04:56] 3 Tech Lead Wisdom

Henry Suryawirawan: So thank you so much for, you know, covering a lot of code review best practices. Adrienne, I have just one last question before we end our conversation, which I call the three technical leadership wisdom. You can think of it just like an advice that you wanna leave to us to learn from you.

Adrienne Tacke: That’s a good question. I think the first one, stemming from my own experiences, is don’t be afraid to switch to something new. That happened to me when I got my student technician job. I had no idea. I was first resetting passwords for people and then it soon blossomed into, why is my computer slow, to how do I install this. It becomes more and more problem solving of a higher level. And I found out that I actually really enjoyed that. And then taking the chance and applying to technical conferences, and actually speaking at conferences and doing that, I didn’t know I was gonna be good at that. I tried it anyway. That’s my full-time job now, and I’m actually pretty good at it. So you never know what might happen if you don’t try it. Especially if it’s something that seems interesting to you, try it and then you can say, oh, that’s not for me. And that’s the worst thing that can then happen.

Second piece of wisdom. I wrote this in a really long article back then, but I, I said, don’t be afraid to fail. I know I said in the beginning, I was afraid to fail. But what I find is if you are unafraid to fail gracefully, I’ll put that, you will not only increase your skills and knowledge in whatever it is that you’re doing, but you’ll actually level up faster because you learn what it is to solve the problem that initially made you fail in the first place. And so instead of being afraid or instead of being like me initially where I didn’t wanna break anything, I walked around eggshells, I stayed stagnant for a while because I just didn’t wanna break anything.

But then the first time I broke something where I actually deleted a couple rows in a table in a database, I was so freaked out. Like, oh no, I’m gonna lose my job, oh my gosh. But that’s where I was able to talk to a DBA to say, hey, this is how you actually roll back something. Here’s how you can do these operations in a staging environment where it’s not as problematic or not as scary. And then here are the ways that you might go through this process again, but do so in a way where you can easily revert back so that you’re not as scared to make changes.

So if I had never made that failure, if I had never deleted those rows, I would’ve just been stuck, you know, doing everything as carefully as possible. And I would’ve never gained that knowledge of environments, of doing things in a way that can be reverted back. And in doing things in a way with more confidence because I know that I can fix it if I need to. So fail fast and fail gracefully.

And then the last thing I will say for tech leadership wisdom. As I’ve gone through my career, I’ve gone through what I feel like is everything. I’ve written a book, I’m writing one now. I’m doing courses. I’ve done the Instagram thing, trying to get followers, all of that stuff. And at this point in my career, I feel like you should have one hobby that you just don’t monetize. I feel like it’s very prevalent in the tech industry to always have another hustle. You’re building a SaaS on the side. You are monetizing, I don’t, I don’t know, building websites, SEO friendly. Like all of these things are cool. But at the end of the day, if everything becomes a goal to make money, it kind of loses its luster. You kind of get burnt out. And I’ve gone through it before where things that were fun, that did become a job because the focus was to try to make money. Just, I didn’t like it anymore. And not only that, you put a lot of pressure on yourself to make it in the tech industry.

And so I say have a hobby that’s just for you. You bake and you eat bread and you don’t post about it. You just enjoy it. You enjoy it with butter or jam or whatever. And that’s your thing. You don’t have to share it with anybody. You don’t have to make money on it. You don’t have to share the recipe. You don’t have to have a bread baking course, like there’s nothing about it. Just have it for yourself. And I think a lot more of us will stay a lot more balanced when it comes to navigating the tech world.

Henry Suryawirawan: Thank you for sharing the last one. I think it’s really, really beautiful, right? So not everything should be monetized, right? Have a hobby that only you enjoy it, that’s also perfectly fine.

So Adrienne, if people want to follow you or maybe contact you for more discussions, right, especially regarding code review, where can they find you online?

Adrienne Tacke: Yes, so LinkedIn actually is a pretty good place. I’m pretty active on there. And then Twitter / X, depending on what you wanna know it as. And then Instagram, I’m also there. So my handle will be @AdrienneTacke, is my first and last name. And you should be able to find me there and I’ll do my best to answer. So please, if you have any questions about code review, I’d be more than happy to answer them.

Henry Suryawirawan: And when are we expecting the book to be published?

Adrienne Tacke: So that just changed. Um, yeah, it’s gonna be summer of next year. So chapter seven is actually going to be released very soon on MEAP. And that’s the chapter on code review comments. So keep an eye out for that. But after that I have 8, 9, 10. I have four more chapters to write and I have a lot of stuff that I want to add to the existing chapters, actually. So summer 2024, keep an eye out.

Henry Suryawirawan: Looking forward for that. I’ve read the MEAP, right, so I find it really, really good. So I think all developers should check this book out. So thanks again Adrienne for your time.

Adrienne Tacke: Thank you, Henry. It was great being here.

– End –