Trust and Ethics in the Workplace
Vitaly Sharovatov will discuss how trust and ethics relate to our careers as software engineers and how fostering them can create more satisfying and productive work.
Ah code reviews. We need them, but we dread them. We do them, but not well. How do we deal with gigantic PRs? Why can’t we write effective code review comments? How do we make code reviews shorter? Is SSDaaRB (Single Senior Developer as a Reviewer Bottleneck) something we just have to accept? In this talk, I’ll not only answer these questions, but give you actionable advice on how to improve your code review today!
I want to tell you:
Let’s change the way we do code reviews. Let’s do better than LGTM ๐ – let’s make the code review processes on our teams great!
Adrienne Tacke is a Filipina software engineer, keynote speaker, published author of the book Coding for Kids: Python, and a LinkedIn Learning instructor who specializes in Cloud Development courses. Perhaps most important, however, is that she spends way too much money on desserts and ungodly amounts of time playing Age of Empires II.
Adrienne Tacke 5:49
Thanks so much, Brian, thanks for inviting me back. Definitely check out those more serverless videos that Brian is talking about. I spoke there a couple years ago, I think and they were always some really, really great sessions. They’re not just because I was there. But usually the sessions that are there are very, very useful, and you learn a lot from them. But without further ado, that’s right, I am here, I’m Adrian Braganza. Teca, I’m here to talk to you about really this emoji here, this emoji that everybody should know or should be familiar with, and what it actually means. In the world of software development, we kind of just take this emoji for granted. And I know for some people, they’re gonna be like, Oh, it’s inverted, it’s the wrong way. It didn’t work with the design of my slides. So we’re using this version that’s pointing this way. But in either case, this emoji either fills software developers, or anybody who reviews or writes or reads code, either with a lot of happiness or with a lot of dread. And that’s because whenever we see this, it’s kind of tied to this notion of ldtm, or looks good to me. And that’s this, I guess, no infamous notion that something passes muster, it’s good enough when we review code. Sometimes we give this emoji as a, okay, you’re ready to go, it’s ready to be merged. And everything looks fine to me, I didn’t see anything wrong with it. The issue is, ideally, this notion we trust, and can fully trust every time that we see this emoji. But that’s not usually the case a lot more often, or anytime we get a, an LG TM, or maybe we’re guilty of doing it ourselves and quickly approving a code review without really going through it, it causes a lot of heartache, and it causes a lot of stress, much like Ryan Gosling here. And so what I want to do about that is talk to everybody, anybody who will listen about code reviews, and tell you what we are doing wrong, there’s a lot that we’re doing wrong, that makes a lot of us really dread the code review process. And that’s a shame because code reviews are a really important part of the software development process. They’re key. And I don’t think anybody really is against them. We know the value of them. It’s just when they’re put into place, and they’re terrible. Everybody doesn’t like doing them. So I want to see why exactly it is they’re so terrible. And that is what we’re doing wrong, essentially in code reviews, then we’ll go through how we can potentially fix those things. They could be a strategy or a tactic that you can apply literally today. And there could be other things that I suggest where it might be a little bit longer, it might take a couple more conversations with your team, it needs buy in from your team, and it needs the entire team to really focus and enforce the things that you’re going to agree to do about the process to make them better. Then once we actually talk about all those things, I feel like that’s the bare minimum of an effective code review. So once we’ve reached the bare minimum, I want to share with you a couple interesting things over the years that I found, worked to make code reviews a lot better and a lot more manageable. We’ll go beyond the bare minimum code reviews that we should be having. So what are we doing wrong? I’m sure there’s a lot of things that are going through your head right now. Maybe you’re maybe you’re watching this at work on the site or listening to me and you’re like, oh, I can tell you. You’re going through it right now. But in order to answer this question, we need to look at it through the perspective of the two people involved in the code review, right? And so we’re going to look at it through the reviewer, and the author’s perspective, because there are things and responsibilities that each have and sometimes we don’t really do the best in those roles. So the first thing we’ll go through are all of the things that we’re doing wrong as a reviewer. And the number one thing that we do is we do not leave our ego at the door, which is really, really unfortunate. So So a lot of the people I’ve spoken to, and in my experience, we’ve always dreaded code reviews, because we see it as this very scary place that we’re just gonna get critiqued that we’re going to, you know, be found out, or impostor syndrome levels skyrocket because someone’s reviewing your code. And not ideally, but typically, the reviewers who are reviewing their code, if they are doing it wrong, are pointing out a lot of the things that are wrong, and they do it in a really unnecessarily mean way. And a lot of the time when we go through a code review, I don’t know if you have a colleague who’s a well actually type of person who kind of holds your code review back because they really nitpick every little thing. They don’t say why it’s wrong, or why something is better or what to do about it, they kind of just use this as a forum to, you know, show how good they are or how experienced they are. And they don’t use the code review as a forum to share knowledge or to help make the code better. They’re kind of just like, well, actually, this is how you should have done it, or this is how I would have done it. And that’s not the case. That’s not what we should we should be doing in the code review. But this is a really, really big problem. And why a lot of people, especially new people, or juniors, absolutely dread the code review processes, because they hate going through a review with somebody who’s like this as the reviewer. Related to that is, as a reviewer, sometimes we focus on the developer rather than the code. So while in the first issue we have, where we’re kind of just like tooting our own horn, and really showing off our ego. The other part is to just focus on the developer and maybe how they may not be writing the best code and really focusing on that, rather than focusing on the code itself. This really digs at the person, this really, you know, is trying to make the person feel less than the person, the author whose code that we’re reviewing. And that’s really, really a shame. There’s a study that was done about this, where they were looking at a bunch of code reviews, and specifically the comments. And they were trying to label the comments whether or not they were toxic or non toxic. And an interesting finding they found was that if you have second person pronouns at the beginning of the sentence, it’s more likely to be considered impolite. So they took a look at all of the different words, they call them Yoona grams in this study, and they divided them up, there were words that were more likely to be in toxic comments, these ones and there were words that were more likely to be in non toxic comments, which are these ones. So if you take a look at the difference, and you take a look at the toxic ones, you’ll see that finding in play that the second person pronoun, are you focusing on the developer, you you’re you are you think, do you think you need to you want the only exception here is the could you in the non toxic section. And that’s an exception, because typically, when we ask somebody to make a change, that is a more polite form, rather than commanding somebody to do something rather than saying, you know, you do this or make this change we use, could you to make it a bit more polite, rather than that all of the other toxic comments typically include this notion of focusing on the developer. Next, I think we’re all guilty of this, I’m certainly guilty of this, and that is skimming through reviews. So one of the unfortunate truths about code reviews, is this. This meme, wow, I’ll call it I don’t want to say that tweet, but it’s the meme. Right? The more lines of code there are, the more likely we’re gonna get that infamous looks good to me, even though we probably didn’t even look at it. And there can be a whole range of issues for why we don’t look at it. There could be I don’t want to say legitimate issues. But there could be pressures of, you know, you just have a deadline to meet. You’re the only person who’s reviewing code for your team. So if you get a review that has a lot of stuff, you’re, you know, less likely to give it a proper review. You have so much on your plate. Maybe you’re about to go on vacation, and you’re like, yeah, yeah, this is my last task of the day. It looks good to me. There’s all these different reasons why we may skim through code reviews. And that’s a really big problem. One of the main themes of why I think we do this is whenever something does go wrong, we have this mindset of, well, if it does go wrong, then the developer has a bug in their code. It’s their fault. And we kind of abstract ourselves away from the code that we just reviewed. But that’s not really what we should be doing. And we should have some accountability there. But we’ll talk about that in how to fix it. But this is a really big problem is skimming through reviews. And there’s also a big part that the author plays and why we may skim reviews. But we’ll get to that as well when we get to the issues that authors create in the code review.
Adrienne Tacke 15:21
And then I think one of the biggest or most important things that we do wrong as a reviewer is to write ineffective comments. So the whole point of us reviewing code is to look at it, probably learn from it, make sure we understand the code. And if we need to actually give feedback, actionable feedback, productive feedback, feedback to make the code better. But apparently, it’s really, really difficult to write effective comments. And so that’s a really big problem with reviewers is writing ineffective comments. Ineffective comments, in my experience, are one of three types. They are subjective, so there’ll be like, Oh, return the result in an array instead? And that’s it. Right? You’re left to wonder, okay, why should I do that, it’s just kind of the feedback I receive is telling me something, which I perceive to be very subjective, because the way that it is going in right now is working just fine. Or may even meet the acceptance criteria, or the design criteria that was created for this particular feature. They are unclear. So this could be better. I’ve gotten way too many of these before. And this kind of starts this really long conversation of like, okay, what could be better? And why would it be better and, you know, you, you prolong the code review process, when you are not clear as a reviewer, because now you have to go back and forth, probably a really long time until you get to the same page about what could be better. And then lastly, if there’s a change involved, or you know, there’s something that might be reworked or refactored, they sometimes like an outcome when you actually ask for them in the comment. So there, again, something like there’s something wrong here. And, again, as an author receiving this type of feedback, we’re left to figure out okay, what did the reviewer mean? What is wrong here? What do I need to change. And those are all the parts of ineffective comments that just delay it that make it really stressful, because now we have to go through and find it ourselves, when really, the conversation could be a bit more clear, a bit more precise, and a bit more productive. So these are all the things we do wrong. As a reviewer, we do not leave our ego at the door, we focus on the developer rather than the code, we skim through reviews and don’t really properly review them. And we write really ineffective comments. So how do we fix those things? Well, number one, forget your ego, full stop. You know, this is not the place to show off how you can make code more cryptic, this is not the place to show off anything, really, it’s a place for you to help your team make the code better. Now, I’m not saying that you’re not supposed to tell your teammates when there’s a potential implementation that is better that you find or an edge case that’s missing. The will the way that you give that feedback is very important, though. So instead of saying I would have done it this way, and leave it at that, you want to say, Have you considered doing something like this, and listing out your reasons as to why that implementation you’re suggesting is better. You know, anytime you give some sort of suggestion, people are going to want to know the why behind it. And so if you remember that and try to focus on giving your better implementations or alternative thoughts like that, and make sure you give some resources to back those up. I really, truly believe that your comments will be better received by the author. And it will not look as though you’re just trying to trample on the code review and show off your smarts or your experience. Likewise, focus on the code and not the developers. So if you really focus on just the code and what is happening and don’t even bring the developer into the mix, that can remain objective, right. And as reviewers, we need to be objective. So if we take a look back at that study where they were determining whether comments were toxic versus non toxic, there’s another finding here that kind of supports that. And that says, We see many software engineering related and Grimms. Basically all the words that were more focused on the code or the software engineering related terms. They were among the non toxic comments rather than the toxic comments. And if we We look back here, we can see that that’s true, at least for this particular study that all of the nontoxic comments, you see tests, unit file files, pull request, you know, they’re not focusing at all on the developer, they’re focusing on the code or something about the code that they’re reviewing. And they are very much objective, they’re focused on that particular thing, rather than the developer. Next, as reviewers, we should be doing proper, thorough reviews. And it’s a combination of us being accountable in terms of giving a proper review, and the author making sure the PRS are actually manageable and reviewable. But from the reviewer side, since we’re still talking about what we can do to improve, we need to do proper, thorough reviews. Otherwise, what’s the point of doing code reviews, if not just to rubber stamp it through. That’s what we’re trying to avoid. And so some of the things that we need to remember here is reviewing code and say 25 to 45 minute focus bursts is one suggestion that a lot of developers are given. Because once you pass the 45 minute mark, at least on average, our head starts to get weary, we become less focused, we are not able to focus on what is in front of us. And that’s comes to anything, not just reviewing code, but especially for something that requires focus, after about the 45 minute mark, we tend to get less focus. Now, again, this is a range rate. And an average some people, that number might be way less than for some other people that might be WAY longer. But the point is review code, when you have a focused amount of time and try to do it in these shorter bursts rather than the really long, you know, I’m, I’m taking about the rest of my day for hours to just review code, that’s probably not going to lead to an effective review for all the reviews or pull requests that you might have to review. Another thing, do not be afraid to send back pull requests that are way too large to review. There are so many times where we kind of just accept it, and we kind of trudge through it, there have been some troopers who are like, well, this is it, like we can’t do anything and they go through 500 files, or a few 1000 lines of code change. And that’s not effective for anybody it’s not, it’s not right to expect somebody to properly review that. And it’s also again, on the author’s part, not they should not expect someone to get a give a proper review on their pull request, if it is that large. So do not be afraid to send back PRs that are way too large to review. If there are logical ways to split it. Ask the author to do that. If you see unnecessary things that are not actually related to the code, ask them to remove that. There are lots of ways to make sure that the pull request is just very much focused on what you’re trying to change. And as a reviewer that is very much within your realm of power to say this is too large, please cut this down to something that I can actually review. Let’s see.
Adrienne Tacke 23:44
All right. Sorry about that froze a little bit. But I’m back. I’m back. Yes, thanks. Thanks for waiting. So part of that is the mindset, right? We always thought at least reviewers when they just pass things through or rubber stamp it, because they just feel like, well, if something happens in production, it’s it’s not me. It’s the developer that has a bug in their code. It’s their fault. And really the mindset that we should be having is how did I miss that? Because as reviewers, we should be accountable to what passes through our review, we should have some notion of ownership of what went through there. Because again, what is the point of the code review, if something does go wrong, and you supposedly reviewed it? So this is less about trying to say whose fault it is or who, who is there to blame if something goes wrong, it’s more about each person has their own responsibility in a code review to make sure it’s the best that it can be. Also, I’m truly a believer of team ownership and team accountability of the code. That way you also kind of remove the ego out of it, and you’re all trying to make the code better. You’re not just trying to make it better for yourself. Have or make it in the way that you would write it. Especially if you’re in a professional software development team. You know that code is not just for you, it’s for the team. And for anyone else who may read this or maintain this afterward, even yourself after a couple of years. So having that mindset of everybody is accountable for this code. And everybody’s responsible for making it maintainable, legible clear. That’s what we need to have in our mind rather than Oh, well, you know, if it’s wrong, then it’s not my fault. And finally, the what I think is one of the most important things as a reviewer, a skill that we all need to have is to be able to write effective comments. So effective comments are the complete opposite of what I said were ineffective, right? Effective comments are objective, they’re as specific as possible. And they have a focused outcome. So being reviewers. I know I mentioned before that we need to be objective, we know don’t talk about the developer, or don’t focus on the Developer Focus more on the code. And there are other things about that, that we need to keep in mind, whenever we suggest some things suggests with facts, right. I’ve have some resources at play. If you have, if I’ve read something, and it’s in a blog post, or you know, on a Reddit post, or somewhere somewhere that you’ve read, or there’s a coding convention that you remember, as you’re reading a code review, and you think that’s relevant, then bring up your suggestion, but make sure to also add, what is supporting that suggestion suggests with facts, because if you just suggest, then it’s more likely to be interpreted by the author as something subjective, something, you’re just saying, well, here you go. Of course, that can vary by team, right? Not every team reads it that way. But the more likely you are, the more likely you suggest with facts, the more likely you are to have the author receive it as something as an objective ask rather than something you’re just saying, for the sake of saying, one of the really interesting things that’s related to this in a different thing is before you ask for a change, ask yourself why you want to make that change. And basically self reflect before you’re about to ask an author to do something to rework something to move something, any type of change the type of feedback that you’re going to ask the author to do. First, ask yourself, because there was a study that was done that says, basically, if you do this, and you spend a little bit of time, just making sure that that’s what you want to do, you come up with two things, either, you really, truly find out that that is a necessary change, and a warranted change. And that’s a good thing, because then when you go to write that comment, and you write that change, you’re better able to articulate that change to the author. On the other hand, sometimes when you think about this, and you self reflect a little bit, you may find out that you either do not understand why you’re asking for that change, and you shouldn’t ask it, or you figure it out, you don’t actually need that change at all. So pausing before writing out that change, even just for a little bit to make sure that it is warranted, has been a very helpful way to make sure all of the changes that we leave in comments as feedback for authors, for the those on pull requests, who received that feedback back is actually warranted and needed. And finally target the code as presented to you. So whatever is in the pull requests, that is what you have to work with. And again, a large part of this is the author’s responsibility to make sure what you need is in the pull requests, but that’s part of being objective is targeting the code, as presented to you in the mechanism with which you are reviewing code. In this case, at least for most of what I talked about in the pull request. And as reviewers, we need to be specific. So I don’t know about you. But every time I get comments back on a pull request, I just immediately assume it’s all stuff that I need to change. It’s all work that I need to do. There’s something that needs to be done for every comment. But more likely, or more often than not, it’s actually just a few things that really need my attention. And others really don’t. They’re more informational, or they are suggestions. They’re not things that I need to immediately address. So being specific about that is very helpful because it allows the author to organize how they’re going to incorporate that feedback and know what is actually important to work on. So one of the things that is important about that as a reviewer that we need to do is to be clear if something needs to be done. So I don’t know about you, but anybody in the anybody watching anybody in the chat, you know, let me know if you use nitpicks or know what a nitpick is. But nitpicks, as you can see, we label a comment with the word nitpick or knit, and then we have Uh, the comment that is the nitpick. And for those that may not know what a nitpick is, these are purely subjective preferential things that don’t make a difference to the code, they’re literally a nitpick, they are something that won’t affect the code. It’s just something that is subjective to you. So we really try to avoid these, we, you know, we don’t want to put nitpicks in the code review in the first place. But if you must, for whatever reason, or your team is okay, with, you know, letting those things in the code review, at least let the people know that it’s a nit. And it’s not something that needs to be addressed. Keeping this in mind, there are other what my team used to called comment signals that we can use to also describe if something needs to be done in a code review. So yes, there’s a nitpick which purely subjective commentary, we try not to do those. But the more important ones that we use were needs change. So if there was something that was a small fix could be updated in a single commit. And it’s just a small update to the current PR, we would leave a needs change comment signal with whatever it needed change. And then if there were larger items that typically required more work than could be done in a small update to the PR, or there’s a lot more conversation that needed to happen. We use the comment signal called needs rework, we don’t put the whole thing there needs rework, let’s discuss offline, that was understood by our team, at least that if we saw needs rework, that meant we needed to have a conversation about it. And so a really, really important part about that is when you do have that conversation offline, and you decide, yes, this does need more work, or no, it doesn’t need more work. And the current PR can go through, there’s just a misunderstanding or miscommunication. What’s really important is to add the results of that conversation back onto that PR, because anybody else on the team that is looking through or may need to go through that history, will now see the full story of what happened with this particular PR. If it was opened, and then close, sometimes you kind of wonder why was it closed? Was it? What was wrong with it, that it needed to be closed? Was it actually addressed somewhere else? These are all the types of questions that we might wonder when we see that particular sign of a close PR, but just by adding a comment on there that says, here’s what we discussed. And here’s what’s happening, it can make all the difference and can help break those pieces of knowledge that gets stuck in between individual people’s minds, and bring that knowledge out to the further team by just having that particular comment on to the PR.
Adrienne Tacke 32:39
Another way people do this for comments is I know, maybe some of you have heard of conventional commits, where there’s a very specific way to format and write out your commit messages. There’s now something called conventional comments. And in here, what they do is a very specific way to outline or format your comments. They have a label, they have some decorators, and they have some particular the comment itself. Well, I like the intent of conventional comments, at least for our team, they were a little bit too much, because they have a list of labels that they suggest that you use. And for us, the lines blurred between some of them, like they have labels with suggestion issue to do chore, praise, you know, praise one is good. But the point was, there were way too many labels that our team would really never really get to use. And the ones that we ended up choosing the needs work and needs change worked for us. But who knows, for some other teams, maybe this level of categorization actually works or is wanted by your team. So this is just another way to not only be clear if something needs to be done, but a way to categorize your comments so that when an author does receive those comments, they know what exactly to do with them. And the last part about being specific is pinpointing exactly what you’re talking about. So there are a lot of code review tools that already have this built in, for example, highlighting only the code that you are discussing. In a code diff, you usually see that what has changed what has been removed what has been added. It’s pretty good about that already that shows but just in case, if you’re able to or you need to do this manually, try to only highlight the relevant code that you are actually discussing, if it helps explicitly call out the method or variable or class names that you’re talking about. Because sometimes in the middle of some complex logic or maybe a larger file, it could get lost and so just calling it out can be again one very nice way of pinpointing what it is you are talking about. And then as another point, which again, it’s might be moot at this point because most COVID utils do this. But if necessary, call out the line numbers to be more succinct. Sometimes maybe you have an entire new function that is added. And then as a reviewer, you only are talking about a couple lines in that function. So, you know, in the diff that might just show the entire function being added. But to be really, really specific, you can say, oh, no, here, particularly on lines 24 to 27. XYZ, right. So the more precise, the more you can pinpoint what it is you’re talking about, the faster the author can get onto the same page as you to know what you’re talking about. And to understand what it is that you are talking about what it is that you may want to change, and what they should be focusing on. Finally, as reviewers, we need to provide a focused outcome. So whenever we do want to ask someone to make a change in their code in the pull requests that were reviewing, focus outcome in that study, code review, comments, language matters here, they took a look at a bunch of code, a code, they look took a look at a bunch of pull requests, my gosh, that word pull requests, and instead of determining whether toxic non toxic comments here, they looked at useful versus not useful comments, which is another interesting study. And what they found here is that those comments that had verbs of a trance, you know, that denote some kind of transformation, those were ranked to be more useful than comments that didn’t have those firms. And so keeping that in mind, I’ve developed something called the Triple R pattern, which people call it the r cubed pattern or their error, Pirate pattern. But this is really, really helpful if you have maybe trouble formulating how you want to ask somebody to make a change, and writing that comment in the most objective and clear way. So what does that mean triple our pattern, you have a request, you have a rationale about it, and you have a result. So the request, what is it that you’d like the author to do? Typically, you can say that in a sentence, and then rationale, a sentence or two explaining why that request is warranted. This is where you add references or links to justify and support that requests, rationalizing why you’re making that request. And then finally, a result a measurable and state ideally, that the author can compare to to see that the change you’re asking for has been met. So this could be a metric to reach this could be a screenshot of what something’s supposed to look like. Or it could quite literally be the code that you are asking for after the change. So a request rationale result. And if we put this through an example, let’s say you wanted to ask somebody to move a method in your review, you can apply this pattern so requests, can we move the Authenticate user method into our library? rationale, we have similar methods that are in that library like this one in this one. And we also call this a few times, which warns its place in the library. Lastly, our team working agreement advises us to place any authentication behavior within this particular library. So you have multiple points of rationale that says, I’m not just asking you this for the sake of giving you more work, it is actually something that will match our coding conventions and match how the rest of our code base is operating. And then the result, right after this change calls to authenticate user go through the library, rather than a standalone declaration that we can use this also to request a more meaningful variable name, really, this pattern works for almost everything. And again, requests can we rename the variable item to something that is less ambiguous? rationale, the, you know, the name is very vague, it provides little context within its scope. And it’s also losing the notion of discounts that’s applied, you know, that’s part of this particular variable. And then result, variable data item is changed to something more meaningful. And then you can even add your own suggestions here of discount eligible item or discount eligible product or something else that you feel is a more meaningful variable name. Now, this is a very helpful structure to create those comments and it’s good to practice with it until you get more attuned to writing comments in this way. So you know, earlier or when you start out if you choose to use this pattern, sure, write it out with the RR you know, requests for a rationale result and write it out in this way. But as your team gets used to it and and starts to see how you’re constructing this change or asking for a change. Obviously, this comment might become a lot Little more condensed, but still has the important parts in there you have the rationale in there you have, maybe you change the result, or you don’t need to add it there because it’s clear, and you have the request. So this is still a very good way to start and practice writing those comments out to be way more effective and way more outcome focused. So triple our pattern request rationale result, you will get very far with that I’m sure when you ask authors to make a change. So as reviewers, those are the things that we should be doing. We should be forgetting our ego, we should focus on the code, not the developer, we need to do proper, thorough reviews. Otherwise, why are we reviewers at all, and we need to write effective comments. That way, we can get our feedback back to the author in a productive and manageable way. So now we know what we’re doing as reviewers. Now it’s time to switch gears and go to the author. And what are we doing wrong as authors as the writers of code as the requesters of reviews, I’m sure there’s a lot that we can think of. But it is a lot, there’s a lot of things that we’re doing wrong, and but that’s okay, that’s why we’re here. And I’m going to tell you what those are. So we can be self aware and ideally, fix them.
Adrienne Tacke 41:20
So first thing that we’re doing wrong as an author, we’re just we’re not being our own first reviewer. I don’t know how many times, you know, when you send an email out, and you just are careless about it, or you just do it really quickly, and you’re like, oh, there’s a typo there. Or, Oh, you forgot to attach something that you said you attach. And oh, you know, you didn’t link to something, there are all these little small things that we don’t catch when we’re just kind of going quick or being careless, or just, you know, not being our own first reviewer. And the same thing happens in code reviews, when we open up our pull requests and ask for someone to review our code. Sometimes we don’t review our own stuff. Sometimes we have a random config file, that’s part of the pull requests that’s not related. Sometimes we didn’t link the work item ticket or use labels, even though we’re supposed to, sometimes we don’t add anything in the description, and we just kind of leave the reviewer to figure it out. And then we get upset when we’re being held up to have our pull requests approved. Because it’s really hard to figure out what’s happening in this PR. So not being our own first reviewer, is a really big thing that we do. And again, similarly to the reviewer, we’re almost like, well, it’s not my problem. You know, once we have opened the pull request, we consider that done right, I’ve done my job I’m done with what I have to do. Now it’s the reviewers job, they gotta go catch everything, I just got to wait for them to finish my review. And it’s done. Not my problem. But again, this is not the mindset that we should be having, we need to have a more collaborative collective sense of ownership and accountability to the code that we are writing. So part of that is us as authors is to review what it is we’re putting into the pull requests and reviewing to make sure everything that is there can set the reviewer up for success to have the most effective review as possible. Next, we do not use any automation during development. So this has been not as much of an issue as the years have gone by. Before when I started talking to development teams and earlier development teams that I’ve been on, this was much more of an issue where when I say automation, there were things like linting, that was not used. There were formatting or lack of style guides that didn’t happen and were not used. There were things like static analyzers, for example, this code will never be reached, that are not being used. And there’s no sense of testing at all that is being used. These are very powerful pieces of automation that helped you as the developer, as the author, fix things during the development stage. And that is the best thing that you can do. It’s also part of you being your own first reviewer is to fix these things when you can. Another part about that is let’s say, I don’t know if you’ve ever gone through a pull request. I have certainly way too many times where somebody just goes through and dots my pull request with like 30 comments of Oh, space, oh, semicolon, oh, you know, little things like that. formatting issues, or other silly low stakes things that should not be discussed in a code review. Those things ideally should never make it into the code review. They should be fixed automatically while you are in development. And that way the things that are brought up in a code review are higher stakes issues, the things you want your reviewer to look for edge cases that you may not have seen some contexts that you may not have been aware of, but they were and they can provide some helpful feedback on the code that you’ve written. These are the things you want your reviewer to focus on, not stuff that can be fixed by automation during development. So the more that you can fix during development and have all of this stuff in place, the more likely you can have a cleaner, more effective code review that a reviewer can focus on more higher stakes issues. And finally, probably the largest thing that we do, largest thing is making our PRs unmanageable. So making them unmanageable is, for the most part that making our pull requests. gargantuan, they’re just gigantic. They are unwieldly, they are things that if you received them as a pull request, you would be probably annoyed that you had to go review this. So PRs that are unmanageable. We’ve already discussed one of those, which is there’s way too many files for you to coherently review or properly review, there are maybe changes that aren’t atomic meaning when you open a pull request, you have maybe two or three different things, or functions or pieces of behavior that are happening that are not related to each other that could be reviewed, isolated on its own singly, that could be easier to review as a single atomic piece of behavior. But no, they’re all together. Or if you do have a slightly larger feature or something else that is have multiple stages, you have those multiple parts in a single PR. So imagine how unwieldly and difficult that might be to focus on a review with all of that in a single PR. These are things that sometimes we don’t think about or just like, Okay, I just need to get my feature done, or I need to get my ticket done, or this is my responsibility. And in the end, we just kind of send it off right or just like carpet not my problem, we did our job and it’s now open in the PR and you just let me know what’s what’s happening and what I need to change. And that actually prolongs the code review process, we kind of are creators of our own demise, when we get upset that code reviews are taking too long. Because sometimes we make it too long to review, because there’s too many things in there to possibly review properly. So making our PRs aren’t manageable. That’s another thing that we do, really, really wrong, and something that we need to be aware of as authors. Another part of that is, you know, one two punch here if you do both. The another thing we do wrong is treating PRs as mystery novels. So sometimes we open PRs like this. Like even our commit messages are just vague, like bug fix bug fix, there’s nothing in the description. There’s, you know, there’s no labels, there’s no linked items, you’re just like, okay, as a reviewer, you’re like, Alright, let me try to make sense of this. Let me go into the diff. Now, yes, there can be easy thing, self explanatory things where you look at the code, and you understand what that means. But if you’re going through the code review process, this is still a form of historical documentation, you still have a record here. And if you’re not using that, to at least put some context in here, or at least be more clear with your title, and explain what it is you you know, you lose out on giving that extra piece of context to your reviewer. And you’d still a little bit more time that the reviewer has to spend to act as a sleuth, right, and figure out what’s going on here. But that is definitely something we do as authors is just be pretty much lazy with our PRs and not add the context that is needed.
Adrienne Tacke 48:55
And finally, something that we do, I don’t want to say it’s wrong, but it can be harmful to ourselves. And that is tying our worth to our code. And that’s really hard. You know, we we asked the reviewers to focus on the code not on us when we are doing code reviews. And yet we tie ourselves to every piece of feedback that we get on a code review, like oh, my gosh, I got, you know, 20 comments, I must be really, really dumb. Or I must have done a really terrible job where my code is terrible. And it’s really hard to untie ourselves, right? We’re only human. But what we need to understand and maybe the mindset shift that we need to think of is even if there’s effective if there’s you know, proper feedback, not the egotistical feedback or harsh feedback, but actual constructive feedback that is left on our pull requests. We shouldn’t immediately feel those as an attack on ourselves or as part of our career identity. We need to view those as a op tunity to improve the code. So that’s one thing that I think would better a lot of people, everybody really that writes code is to not tires worth to the code that we write. And those are all of the things that we do wrong. As an author, we are not our own first reviewer. We don’t use any automation during development. We make our PRs really unmanageable, and possibly even treat them as mystery novels for the reviewer to solve. And we tie our worth to our code. So how do we fix those things? Well, first things first, we should be our own first reviewer. The great thing about being our own first reviewer is that we can do it as many times as we want. So whether that’s doing it after every commit that you do, or maybe you even open what’s called a draft PR and have that be, you know, your staging area for yourself, where you make sure everything is ready before you actually submit it to be properly reviewed, or officially reviewed. Whatever mechanism you can use to look at your own pull requests and make sure everything is ready to go has all the context that it’s needed. Be your own force reviewer so you don’t miss all of those little things. So you don’t and you catch any small issues that can be fixed and don’t have to prolong the code review any further, just because you’re a little bit careless or acting too quickly, when open learning the pull requests. Next, let the robots take over during development. Again, they are way better at it anyway. So you know, if your team has a style guide, or you do have linters, and you have rules in place that can help you enforce your coding conventions, use them, or at least start talking to your team to develop those rules or develop that style guide and have it enforced properly for everybody. Ideally, that should take care of all of the debates about styling and formatting. If your team is able to agree on a style guide, right, but if you do have that, use them use static analyzers if you have the ability to use them. And if you have tests, make those a priority, run those automatically, after every commit or after every saved is again, this is part of being your own first reviewer. But this is also a, I guess, a mindset thing where I’d rather have the computer or robot, I’d rather have the computer tell me that I did something wrong, something silly. And let me know that I need to fix it. Rather than receive that same comment in a code review. Rather than have a colleague tell me to fix it, I will see all the squiggly lines or errors or warnings in my IDE all day. And I will fix that much more happily than having to do it through a code review. And if you have that mindset of just trying to get it as clean and as ready as possible, then you know that the the code review that you are getting, will likely focus on more higher stakes issues and not lower stakes issues that you yourself could solve with automation. Be your own first reviewer, right? Feel like I had that already once. Yeah, so be your own first reviewer. Yeah, I went backwards. So that’s why I confused myself. So be your own first reviewer use automation during development. And then the next one was to make your PRs manageable, aka, make them smaller. That’s really, if that’s all you remember about this is try to make your PRs as small as possible. So manageable PRs, they focus on an atomic change or feature, right, the smallest common denominator or lowest common denominator, the smallest amount of behavior that you can isolate into a single PR is ideal. Because if you can focus on that, then it’s not only going to be a smaller PR, but it’s going to be less of an excuse for a reviewer to say, Oh, I can’t review this, because I don’t have time, include only what’s necessary for the code changes. So work to work. So again, if you have random config file changes there. If you have, you know, console log statements or you get you had a Git stash that you popped in, it’s accidentally, you know, adding some extra behavior into your pull request. There are all these unnecessary items that somehow get trapped in our PR, make sure that only what is necessary for the code changes you actually want to have reviewed are in the pull requests, split multiple parts into multiple reviews. If you can proactively plan this out when you know that it’s either a multi phase feature or it’s a larger feature that has dependencies. You can work with your reviewer, have a discussion about it, say this is how we’re going to do it. And I’m going to first put this particular part in one PR and then we’re going to walk it through with these other two dependencies. But make sure you just split multiple parts as logically as possible into multiple reviews. Manageable PRs, also likely can be reviewed in 10 to 20 minutes. So this is a again, these are all rough guidelines, right? Don’t just take this would be like Adrian said, this has to be reviewed in 10 minutes. But if you take this as a guideline, you then remove this bottleneck or excuse of reviewers now saying, Well, I don’t have the time. Well, if it can be reviewed in 10, to 20 minutes, you have the time, you can also easily quickly check what is happening in that pull requests. And if it’s focused on some single thing, and it’s that small, then there really is no excuse for someone to just review your code, especially if you’ve properly created it to be as small as possible have all the contexts that you need, and is just a very easy pull requests to review. Now, there are some other things that support what a manageable PR can be. There’s a study by Dragon Stepanovich, he says typically, manageable, manageable PRs are under 500 lines of code. And in the study that he did, he says this is due to the fact that if you have less than 500 lines of code, you actually receive more comments. Now, you might be thinking more comments, that’s not a good thing. But in this in this context means that if you have fewer lines of code, the more engaged the reviewer is, and if the more engaged the reviewer is, then you might have more comments. But that’s what we want. We want actual engaged reviewers to actually go through the code to actually read through everything that’s in the pull request, not just skim it or not look at it, or say yeah, it looks good. And then there’s actually an issue there that could have been caught in a review. So if you have under 500 lines of code, Dragon suggests that this would be a more engaged review, and would be a manageable review. And finally, another rough guideline, there’s a different study by Microsoft that took a look at how their employees, their developers ran through their code reviews. And they found that the most manageable ones were typically less than 20 files changed. So that was about the cut off for where it starts to get unwieldly unmanageable or wait too overwhelming for somebody to review, right at that cutoff line. If you can keep it under 20 files change, it’s much more manageable, there’s less cognitive load for the reviewer to make sense of everything. And it’s much more likely that they can review it in a proper, thorough way. So these are all rough guidelines on how to make your PR way more manageable.
Adrienne Tacke 57:47
And finally, solve the mystery. Let your PRs tell the story. Don’t let your reviewer do all the footwork and guess at what it is that you’re trying to have reviewed. So there’s a quote I like to think of about this, it’s when one is writing a letter, he should think that the recipient will make it into a hanging scroll. And that’s by Yamamoto, Sumitomo. And when I think about this, quote, I think about code review, pull requests, because they need to be full of context. Code reviews are again, they’re they’re there to be a historical, or they produce a historical artifact for your team. And so if you keep that in mind, and you have your pull request, be full of context and decisions. And, you know, information about why your code was changed at this point in time to the way that it is, then it’s much easier for you to look back and say, This is why we got to this point in our code base. So if you have a clear title, if you have the context necessary in your description, if you have commits that are atomic and also descriptive, if you have labels that are actually used, and you appropriately assign them, if you link tickets to the, you know, initial criteria or ticket that started or originated or invoked this work, all of these pieces of information are now there for the reviewer to say, Oh, this is why this code has changed. This is the context with which it needed to be changed. And this is why we chose to do it in this way. Compare that to earlier when we didn’t have any of this and we didn’t have any context to work off of well, then the reviewer can just have to guess and make up their own context of why the code was changed the way that it was changed. So point being give your reviewer the information that they need to make the best not guests, but review within the right mindset for your code. And also leave your team with a really good, detailed piece of historical information about your codebase and If you are going through the code review process already. And finally, finally, remind yourself that you are not your code. So, you know, again, we’re really, we probably self identify, especially in the tech community that this is what we do, we need to be really good at it. And that means that we, you know, are a good person or that we’re supposed to be in this role, or, you know, all of these different things that we tell ourselves and that if any thing different comes to us than what we’re thinking about it, then we start to feel upset at ourselves. What we need to do, like I said before, is have a collective sense of ownership and accountability to make the code as best as it can be. And if we remind ourselves that we’re not our code, then we are in the right mindset to accept productive feedback, we are not here going, Okay, well, now I’m going to be attacked. Let’s see what I did wrong. No, we’re here receptive and open to the feedback that could make our code better. Or maybe there’s something we didn’t miss. And we didn’t take into consideration. Maybe there’s someone that has more experience, or has a different piece of context that we may not be privy to, that is now brought up into the code review. And now we have more knowledge to say, oh, okay, maybe that actually plays a role in how I should have implemented this. That’s why we have a code review, right is to find ways to make the code manageable for all make it better for all. And so if we remind ourselves that we’re not our code, then we can shift our mindset to be more receptive and less reactive or angry when we receive feedback on our code. And those are all the things that we should be doing as a reviewer, we need to be our own first reviewer, we need to use automation as much as possible during development, we need to make our PRs small, as small as possible. And we need to make sure that we have as much context as necessary in the pull request itself. And finally, we need to remind ourselves that we are not our code.
Adrienne Tacke 1:02:09
So now we all know what we’re doing both reviewer and author, it takes both to understand their responsibilities. And we both need to be accountable for those responsibilities. If we are to get to an effective code review, one, that should be the way that it’s supposed to be not what we remember or understand of a code review. But one that is enjoyable, one that doesn’t make us hate our team members or hate each other one that doesn’t make us dread the process itself. One that we know the value of and is effective and as manageable. And as enforceable. That’s what we need to do to make code reviews, the key process that it needs to be on our teams. And that’s the bare minimum, right. So if we are doing all of those things, maybe your team is already doing all of those things. And that is awesome. That means you probably don’t hate or dread your process as much as other teams. But once we get to that point, and we have pretty decent code reviews where they’re doing the job that is set for your team or achieving the goals that your team wants it to. There are now some things that I want to suggest that may make you go beyond the bare minimum code reviews. So the first important thing there is to create a team working agreement. We, I alluded to it earlier in the triple our pattern. But this is something that is so key that I think every team should have it and this is a document, it could be a markdown file, it could be a Google Doc, it’s just needs to be some accessible file that everybody has access to that can easily be seen. And it’s a listing of what the team has agreed to in their process. So for example, what is the code review process? What is the definition of Done? Do we need two people to approve? Or three? What happens in an emergency situation? Can we bypass the code review all of these things that we kind of not take for granted. But we all feel our common sense things. Those are different between the different team members of a team. And so if we just kind of expect or assume everybody is on the same page as us of what those expectations are, then you’re in for a bad time. So if you take all of these implicit expectations and make them explicit on this document, then everybody is on the same page, then everyone knows this is the process. This is the definition of Done. These are things that we are allowed to block pull requests on and these are the things we’re not allowed to pull to block pull requests on. These are all the things that either start debates or start arguments or prolong the code review process, all because everybody had a different assumption of what The right thing to do within the process. So if you create a team working agreement, you talk this out with your team. And you agree to what your process is and what the different pieces are, then everybody works from the same playing field, you are all aware of what the process is, you can use this as a reference or as a point of rationale, right? If you have something that’s explicitly stated in the team working agreement, and something is not going, you know, something is going against that in the code review, you can reference this to say, look, this is what we agreed upon. And this is how we’re going to deal with the issue at hand. The other great thing about this is that once you’ve written this or created this agreement, this is a living document, it’s not something that’s set the once it’s been created. So if you find that a part of the process is not working for your team, or your team evolves, you can change it, you change it with the processes of your team, you change it to work for your team, what’s most important about it is that everybody is discussing what the potential change is, and everybody should agree or at least try to come to an agreement for what’s about to change. So having a team working agreement is a really, really useful thing to have, and is a thing that you can evolve with your team as you go. But it can be very, very useful in the context of code reviews, if possible. Another thing you can try are PR templates. So there’s this notion of templates where they’re, I guess, semi automated. But for example, in GitHub, you can create a template that says if this particular type of pull requests is open, then show this template. It’s like a checklist or a description. It’s this Markdown file of all the things that you want the author who’s opening the pull request to view. And this is really helpful for authors to make sure that we consistently get the information that we want in our pull requests. It also helps them to not have to remember everything we might ask for in a pull request. So for example, there is a bug fix PR template that is here. And if anybody opens up a pull request that is a bug fix type, they could have a template like this, that’s what is happening. Give us a sentence or two of describing the bug, tell us the reproduction steps to get this bug to show. And then what was the root cause that you found? What was the actual fix? Why did the particular fix that you’re submitting right now solve this problem? Is there a testing strategy that was used? How did you actually make sure it’s truly resolved? And are there any implications that this fix may have? Will this introduce other issues or even cause more bugs? Same thing regression risk? Are you really, really confident that this does not introduce new issues? If so, did you take any steps to mitigate that risk? And the thing is, those are all really nice things to have. But your templates can also be something as simple as this portion right here, which is a checklist that the author self acknowledges that they have done. So yes, things you know, sometimes that we forget. Yes, we’ve linked the issue. Yes. We have tested this locally. And it works. Yes, we have added new unit tests, and they’ve been added to prevent future regressions, or, yes, we’ve updated the documentation. So yes, these are all self acknowledgments. But the thing is, if the author is going to say that they have done this, and something else is wrong, there is a point, an objective point here to discuss that says, Well, look, you said that you put this in the checklist, we expect everything that goes through to acknowledge this checklist. And really, this is again, not to blame other people. But it’s to enable the developers opening up a pull request to not forget anything in the PR and to make sure it has the description and context needed for a proper review to take place. And the cool thing about this is that you can have a different PR template for different types of issues. So you might have one for a bug fix like this. Or if you have a new feature, you may have a different PR template, then you would want to have different items in a new feature template versus a bug fix. Or if you’re only updating documentation, you may ask different questions or want the author to double check that they have done particular things for documentation. So PR templates are a really nice tool to have or tactic to use to make sure you get that information that you need in pull requests.
Adrienne Tacke 1:09:39
One really, really neat thing that I think most people should be doing already is to auto assign reviewers. So sometimes code reviews take a while because you don’t know who to assign it to, especially if you’re working on a larger team or and code bases affect multiple teams. Maybe you’re new and you don’t know that about This person is or maybe you just want to make sure you get the best reviewer possible for your pull requests. So, auto assigning reviewers takes care of this. But it also takes care of what I like to call bias or buddy reviews where you know, it’s the same two people, they just open pull requests, and they asked each other to review it. And they always get it approved, because it’s, it’s a buddy system, right. So that helps with all of those things, auto assigning reviewers, make sure you get the best reviewer possible. So in GitHub, this is called a code owners file where you have a file that can do pattern matching. And for every file that is changed in the PR that matches that pattern, it will auto assign whoever you added into that file, for example, if you have, you could write a pattern that says any JavaScript files that are changed, so Astrix dot j, s, any JavaScript files that are changed in this particular code base, if a pull request is opened, and files are changed that match this pattern, then assign Adrienne to this pull request, or you can also do groups. So you can say front end group or whatever it is that you want to assign the reviewers to. And the same thing works in Azure DevOps, you can auto assign reviewers via a reviewer policy. So if you go to your branches in, in Azure DevOps, you’ll find it really, really low. On the very last thing here, it’s the automatically included reviewers policy here, it’s the same thing, it acts the same way as a code owners file in GitHub, you just gotta go through the GUI to put it but same thing you can say these are the reviewers you want to add, you can make it a required policy. And this is the where you put the pattern matching for how you want to decide who is assigned to what pull request. So auto assigning reviewers is a really neat thing that you can do to help out with bias to help out with getting the best reviewers assigned possible and to do it in an automated fashion. And then the last thing is something called informational reviewers. So we’ve all heard of things like auto assigning reviewers, or you know, how many reviewers do I need to approve, ideally, two or more. But informational reviewers is something that I’ve tried with my team before. And this is where you have reviewers who are assigned to a pull request. But they are informational. They see what’s going on in the pull requests, they acknowledge that they have looked at the pull request and have taken seen, you know, any discussion that has gone through it, but they are not necessarily required to approve the pull requests to go through. What this does is it adds more people to the eventual pool of reviewers that can approve. And what this was created for was I’m sure a lot of people can relate where there’s like the senior developer as a bottleneck service type of thing where there’s only the senior developer can review every single pull request. And then they get overwhelmed because they’re the only ones that can review everything. And then code reviews take too long, because there’s only a single person that can review everything. So in order to cut down on this bottleneck and to increase, eventually the pool of people who are knowledgeable and can approve you add informational reviewers. And the idea behind this is to level up and have them be come more aware and share that knowledge, absorb that knowledge from the pull requests that happen with the pieces of the code base that they may be unfamiliar with at the moment. And the more that you do this, and the more informational reviewers you add and the more that you go through this, the more that they can be aware of what is happening in the codebase. And the more that they can eventually be able to feel comfortable and be knowledgeable about being able to approve certain pieces of code or review pull requests that affect pieces of code that they may not be aware of before. So this takes care of the thing where it’s like you assign it to somebody, and they’re like, oh, I have no idea about this. I can’t review this. So when are you going to know about this piece? Right? If you are somewhat expected or can be believed to look at this code or maintain this code in the future? I truly believe that you should be aware of what is happening and have some sense of what’s going on there. And so yes, at the moment, you may not know anything say about this piece of legacy code. But if you continue to be part of pull requests and see what’s happening, and see how people review it who do know and are aware err, the more that you level up your knowledge and are able to share that across the team spread it across the team. And eventually the pool of reviewers does become wider because more people are now comfortable and are aware of what’s going on in those parts of the codebase that tend to only get reserved to a single person. So informational reviewers are something to explore. The last thing I’ll say about this is that there was a development team in Germany where I spoke and they actually were doing this, which was rare, I always ask people if they have done this or have heard this notion, and they actually do informational reviewers, but they do it to a whole nother level. So they do have informational reviewers where they add them. And they’re not initially not necessary to approve, they have different reviewers for that. But if you are an informational reviewer who looks at this, you actually do have the power to approve, even if you’re only an informational reviewer. But they do this with the expectation that if you approve as an informational reviewer, and something does go wrong, you will be the one that’s expected to fix this. And they do that because now if you want to have that ability to approve, or you want to be accountable to be able to approve that particular pull requests, you they found that their developers were more likely to ask questions of the author to really research that part of the code to talk to more of their colleagues to really understand what is happening there. And finally, learn what is going on in that part of the code base so that they too can be knowledgeable. And they do that to the extent that if something did happen, they would be comfortable fixing something that was approved under their hand. So that’s another take on informational reviewers that I thought was super interesting, and also gives a lot of people people the availability and enables them to participate much faster in a code review and give them that that power to be part of the team and contribute in a faster way. So last thing I’ll say is that code reviews are always thought of as this critique thing, right? It’s always negative. It’s always here’s what you did wrong. And we never think about code compliments. So I do encourage code compliments, but within reason. So this is not to turn this around and say, you know, for every little thing, you’re like, good job, or this is great. Or oh, you know, like, I know that I’m not saying to be less happy or less supportive of your teammates. What I am saying is if you find something really novel, like there’s a bug that no one has been able to solve for a few weeks, or there’s a really, really clever optimization that somebody has done or just something that has really impressed you then say so write a compliment and tell your colleague that you were really impressed with it and that this is awesome, and that you value that they were able to come up with the solution. Doing this within reason and doing it every now and then makes those comments so much more impactful. And they obviously make the team much happier because they see when ever they do have a clever solution to something that it is acknowledged. So compliments are definitely encouraged within reason. And with that, I will say to anybody that is listening, please do not settle for a code reviews don’t continue on as as usual. Do not settle for the process that you currently have or may not even have and just live with it and dread it. Strive for the great ones strive for ones that you can enforce strive for the ones that achieve the goals that your team wants it to and strive for the ones that you know that it can be strive for really, really great code reviews, and your team will be much happier for it. So with that, I’ll say thank you CFE.dev If you like this talk that you’ll love my book. This is all from the book I am currently writing looks good to me constructive code reviews. And actually the last chapters just dropped on my book so if you are interested in it, definitely give it a look.
Brian Rinaldi 1:19:26
Thanks, Adrian. That was amazing. Other leads us up so good morning. QR code and and congrats on actually getting those last chapters. And I know I haven’t written books before it’s it’s quite the journey. Yeah, yeah. I’ve said flat out like, I will, I will probably never do it again. I say probably, right. But yeah,
Adrienne Tacke 1:19:53
it’s what we say. Yeah, no,
Brian Rinaldi 1:19:55
I know. Last time I said that and then I got talked into it. You know, this time I’m more serious. I’m more serious this time for real. Okay, so I know we were kind of overtime. But I just add a couple quick questions that mean you. Honestly, I even mentioned in the comments of like I want I strive for the kind of clarity you have in your slides. And so, kudos to you on that. Quick question. You mentioned something about tools for code reviews. Do you have any that you recommend? Specifically, I know, like me, most of the code reviews I’ve been a part of were just in GitHub, like PR comments, and I often find they were hard, when there was a lot of them to like, kind of find everything. And then we’d always end up with a cycle of like, Oh, you missed this one. I missed that one. Because some were, like actionable, and some weren’t actionable. And then like, so it was hard to figure out where, you know, when I was done, do you have any tools that you recommend beyond that?
Speaker 1 1:20:54
That’s a great question. I think this depends on the enforcement of the team, right? So if you are all actively, let’s say, labeling your comments in the way that’s easy for you to categorize them, that’s one way to do it. You can also use some GitHub actions, but that’s more along the lines of like pre build steps, rather than the actual pull requests itself. What I will say is, because it’s AI, the age of AI right now, right? Of course, I had to research, you know, can AI just solve? Can they can they just do it for us can do even have to be involved in the code review process at all? And no, don’t do it. Just like most things, now, with AI, you can certainly use a AI based tools. So for example, there are AI based tools that actually write a summary of your pull request for you. So if you have trouble doing that, that could that can be one way to make sure you have the description in there, there’s actually a different AI tool, I think it’s called Code rabbit. That’s the most promising one I’ve seen so far. Where it reviews your pull requests, it reviews your code. And then you know, for the people who are like, Oh, my reviewer takes forever to give me feedback, you can actually start a chat with it on your own pull requests and start discussing any feedback that the AI has found. So it can leave comments on your pull requests. And if you pull on those comments even more, continue to ask for clarification. You’re basically now talking to an AI reviewer at that point. So I’m still weary of that right, obviously, use it as a first pass worse, I’m not saying do not use AI tools, because they definitely will make some parts of the code review faster or more efficient. What I will say is, and this is still my opinion, it has not changed yet, use them as much as you want. We still need humans for nuance, and context. And all of the other things that AI or in general computers can’t understand. And one really good example of this is like, you know, let’s say you ran a code review, AI tool, and it left you pretty decent comments, right? All these things that it can find, there could be like if you left a comment that said, Write a more meaningful variable name. At this point, there’s no AI tool that can determine what meaningful is. So you could leave that feedback or it can try to check for that feedback. But you still need a human to understand what is meaningful mean in this context in the context of your team in the context of this codebase. So that’s what I’ll say about that. It’s definitely up to the team to enforce all of the tactics that I’ve said to make it more efficient. But do not be afraid to use the AI tools either just know that it’s there to augment, not replace.
Brian Rinaldi 1:23:53
And so one other just real quickly, I’m assuming one of the things that kept coming to mind and you give some tips on it, but like, things like email, like ego and other things like that, that are more trying to kind of change behavior. Do you have like tips in your book on how to operationalize some of these things? Like how do we, how do we enforce something like, you know, people kind of making changes that say, are more ego driven than actually be like, you know, important to, to the code?
Adrienne Tacke 1:24:28
That’s a great question. A lot of this does depend on enforcement from not only the team, but from, you know, those above the team. So that’s one part of it. In order to automate it, that’s a little bit harder. The one thing I can think of at this moment there is there are tools that can review your pull requests, for example, and make sure you’re using inclusive language. I think there’s a Microsoft inclusive static analyzer. That’s one way to do it. In turn, As of the AI tools, that’s also now being built in where if you have, if you as a reviewer start to write comments, it can kind of pop up like a Grammarly pop up, like for spelling and grammar. But in this case, it’ll say, you know, this can either be written more concisely or clearly or this tone actually sounds a little bit passive aggressive. So maybe you might want to rewrite it, or it will give you a suggestion on how to just be more objective and clear rather than the tones it might be finding. Now that one is still very much like, in, I would say, alpha stage, it hasn’t gotten some of the comments, right. So this is again, why I say it’s hard to at this point, operationalize it, it really is dependent on the people. But those are some of the off the top of my head, how you could operationalize some of those things. But it really does come down to making sure the team is on the same page. And you know, wanting to not be a jerk to each other. Like, if there’s somebody who let’s say, You’ve, you’ve done the team working agreement, you know, you’re all for all intents and purposes are working just fine. Everybody’s following the process. And you have one person that just happens to not want to follow that or not want to adhere I don’t I’m not I don’t want to say like, follow the cult or anything like that, or fall in line, but just be a part of the team in the way that you expect them to. Then what typically happens is that person doesn’t last very long if upper management supports the team experience and supports the overall team being happy rather than, say protecting one, you know, superstar developer.
Brian Rinaldi 1:26:45
Yep. Okay. And I think the the other suggestion, maybe next time you pretend to be slick. What you do is you buy the whole team, the book,
Adrienne Tacke 1:26:54
or that? Yeah, certainly. I’ve heard a lot of. Yeah, I’ve heard a lot of people say how you know what, I’m gonna give this to my colleague, because they need to listen to this or about this talk. They’re like, do you have slides? Because I just need to send this without any words just here. Read this. So.
Brian Rinaldi 1:27:13
Okay, well, I really appreciate you presenting to us. This was like super informational.
Vitaly Sharovatov will discuss how trust and ethics relate to our careers as software engineers and how fostering them can create more satisfying and productive work.
Join Erin Mikail Staples as she walks you through the using Discord to manage developer and technical communities.
James Quick will share tangible advice he's learned over the course of his career that can help you bring your career as a developer to the next level.
Join 12 fantastic women speakers for exciting talks on web development, JavaScript, tech culture and career development.
Join 12 fantastic women speakers for exciting talks on web development, JavaScript, tech culture and career development.