Hey, a quick message from me. Thank you for being part of the Techledjunal community. This show wouldn't be the same without your ears and you are the reason this show exists. Creating this podcast is a labor of love, but the truth is it also takes time, resources, a whole lot of passion, and an extra bit of caffeine. So if you're loving TLJ and want to see it keep on growing, consider becoming a patron at techledjunal dot dev patron or buying me a coffee at techly
journal dot DAV coffee. Every little bit helps field the research, editing, and sleepless nights that go into making this show the best it can be. Thanks for being the best listeners any podcast could ask for. A lot of developers tend to tie their self worth to their code. Sometimes we feel like our code is an extension of ourselves.
So being able to let go of your ego and understanding that if we're asking the reviewers to objectively review our code, the feedback is based on the code as I have presented it has nothing to do with me, has nothing to do with anything about me.
It's just the code. Hey everyone, my name is Henry Surya Virawan and you're listening to the Technically Journal Podcast, the show where I'll be bringing you the greatest technical leaders, practitioners and thought leaders in the industry to discuss about their journey, ideas and practices that we all can learn and apply to build a highly performing technical team and to make an impact in your personal work. So let's dive into our journal.
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 Tag. Adrienne is a software engineer, keynote speaker and the author of the upcoming book looks good to me. In this episode, we discussed code reviews and why it is an essential part of the software
development process. Adrian discusses the importance and benefits of code review, the common code review workflow, and the different roles involved, how to provide effective code review commands, and why we should leverage on code review tools and automation. She also provides tips on how to speed up our code review turn around 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 faster rating and review on Apple Podcasts and Spotify. Let's now go to my conversation with Adrienne. Hey everyone, welcome back to the New. Episode of the technicianal podcast. Today I have with me Adrienne Tech. She is actually writing a book, which is something that I look forward to read, 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, Adrian, welcome to the show. Thank you, Henry. Thank you so much for having me. I'm excited to be here. Right. Adrian, I'd 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. 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 Two or Age of Empires 2. 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 Ave.
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 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 Ave. 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 is sharing or giving the talk was really, really bad. I mean, I wasn't really learning anything. The slides were just bullet points. I 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, 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, OK, 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 the, 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 7 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, nor was I expecting to be, you know, to actually really like speaking
on stage. I'm usually 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. Thank you for sharing your story. I think it's really interesting. You highlighted 3 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 step and probably look at you now, right? It's partly your main life now. This episode is brought to you by Miro. Collaborative development is one of the crucial. Aspects of Software Development and Code Review is one such core activity that can benefit from a good collaboration.
As you will learn in this episode later about Code Review best practices, finding a collaboration tool that can assist code review process is extremely useful, especially if it can be done in a fun, engaging and collaborative way. One use case that I find mirror extremely helpful for code review is for diagramming and process mapping.
You can use it for any of your technical diagrams such as Class Diagram, Sequence Diagram, Infrastructure and Architecture Diagram, Data Modeling Diagram and so many more. With a lot of readily available predefined templates, you can kickstart your collaboration within seconds. It also has support for popular diagram as code tools such as
mermaid and plant UML. Explaining complex ideas and systems becomes much easier now and as a result it will lead to a higher quality work and most importantly more engaged and productive software development teams. Find simplicity in your most complex projects with mirror Your first three mirror boards are free when you Sign up today at mirror.com Podcast. That's three freeboards at miro.com/podcast. And now let's get back to our episode. So today we are gonna cover this
book title. 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 why you are writing this book? Great question. So LGTM, the infamous acronym, or some people use the thumbs up emoji, right? With that. This is usually used to say everything looks, you know, OK, 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 it's 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 DIF that shows the differences between the changes and what has been done. Sometimes 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 OK, 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 it'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. 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? The 1st is about PR, Mr. or CR right? When you set it can also be done through pair programming right? It doesn't mean that code 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 a branch before they can merge. Any kind of maybe fun stories or horror stories that you have experienced with code reviews that you can share with us? Yes.
So my very first experience was actually with Team Foundation Server, 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 that 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, yeah, 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 first stand 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 is no such review process on my own teams. 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? 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? 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. It's something like, oh, this could be better, and then that's it. That's the only comment you get. And you're like, OK, 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 #2. 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. O There are a couple of examples in my book where I share these are on 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. And 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 going to go around it. So any process that is made much more difficult than it needs to be without any additional value, developers are going to 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. 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 commands 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 upscale
ourselves. 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? Yeah. So you've been 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 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 going to 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 1st 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 DIF 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 request or the change request. 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, oh, 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 wouldn't 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 get 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 web hook 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 in a nice way like that. 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, common 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. So I want to tackle the other topic that you cover 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. 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 want to 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're 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 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? Yes, not only PR, sometimes the code commit command as well.
Right. Yep. 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? It 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 someone 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 5000 files changed or 8372 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 could do it in 10 minutes, it's going to 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 route, right, versus having like a gigantic pull request with several changes muddled in there along with configuration changes and some package, Jason 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. 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 and we just submit the Pi 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, look at the number of changes and make it more manageable, right. So I think making it more manageable to me is maybe one of the biggest important thing that we can do in order to make it less dreadful. So now the author has created the PR, so there's a reviewer. I think this is also another important role.
So tell us the responsibility for the reviewer. 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 Co worker, 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 egg shells. 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 want to 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 going to ask someone to make a change or you have a request or something, I say the three Rs. 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 going to 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 2 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 they're 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 1020 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 asks 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 going to make that request, then you're going to 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, 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. Yep.
Also, you mentioned something really, really important that I want to highlight here as well, right? Eventually, the reviewer also responsible for the code that gets through right to the production. 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. 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. 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 ask senior to review your
code, right? You kind of like just wait and kind of like back 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? 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 #2 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 going to change, your code base is going to change. Your project's expectations are going to change. There are a lot of things that are going to 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 going to go the happy path, right. It's not just we're going to change some code, we're going to review it, Some people look at it and then it's goes into the rest of the code base. There's going to 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, OK, 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, 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 OK 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.
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 liking, 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. 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. So you 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 and 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 first hand 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 build check. There are other tasks that you can have read also when you open APR. 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 OS 10 issues that are glaring that somebody else may not have the time to check? 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, APR 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 APR 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 request 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 APR. 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 that 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. I think if as a team right you manage to remove all these accessories thing, I say like formatting, spacing, comma here and there, right? I think that's a good part that everyone should aspire. 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. Yes. So aside from the really good pattern I shared, which is the Triple R pattern, the you know, request rationale results. 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 want to 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 that I get a little bit worried. I'm like, oh, I got to 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 it's 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, as 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 #2 to make sure it's actually saying what it wants. You know, the intent of what you actually wanted to say to make sure that's still in there, so you don't want to 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. 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. 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 and 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. 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 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, fifty 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 going to 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 going to 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 XYZ 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 have 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.
Think of how much headache you would have, 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, airing 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. Right.
And I would also say that make it as part of the work, right, make it as part of the definition of done so that you kind of 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 expectation explicitly. So thank you so much for you know covering a lot of code review best practices.
Adrian, I have just one last question before we end our conversation, which I call the three technical leadership wisdom we can think of it just like an advice that you wanna leave to us to learn from you. 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 going to 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 that 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 1st place. And so instead of being afraid, or instead of being like me initially where I didn't want to break anything, I walked around
egg shells. I stayed stagnant for a while because I just didn't want to 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 going to lose my job, Oh my gosh. But that's where I was able to talk to ADBA 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 have just been stuck, you know, doing everything as carefully as possible. And I would have 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 SAS 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. 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 can enjoy it?
That's also perfectly fine. So Adrian, if people want to follow you or maybe contact you for more discussions, right? Especially regarding code review, where can they find you online? Yes, so LinkedIn actually it's 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 Adrian. Teca is my first and last name and you should be able to find me there and I will do my best
to answer. So please, if you have any questions about code review, I'd be more than happy to answer them. And when I'll be expecting the book to be published. So that just changed. Yeah, it's going to be summer of next year. So Chapter 7 is actually going to be released very soon on Meep and that's the chapter on code review comments.
So keep an eye out for that. But after that I have 8910, 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 a summer 2024? Keep an eye out. Looking forward for that, I've read the MIP right, so I find it really really good. So I think all developers should check this book out. So thanks again Adrian for your time. Thank you, Henry.
It's great being here. Thank you for listening to this episode and for staying right until the end. If you highly enjoyed it, I would appreciate if you share it with your friends and colleagues who you think would also benefit from listening to this episode. And if you're new to the podcast, make sure to subscribe and leave me your valuable review and feedback. It helps me a lot in order to
grow this podcast better. You can also find the full show notes of this conversation on the episode page at techlitjournal dot dev website, including the full transcript, interesting quotes, and links to the resources mentioned from the conversation. And lastly, make sure to subscribe to the show's mailing list on techlitjournal dot dev to get notified for any future episodes. Stay tuned for the next Techly Journal episode, and until then, goodbye.