Hello everybody, this is episode number 85. You know it. I know it. Zorak know it. This week we’re gonna be talking about the code review process for Web developers and how it can make you a better dev. Ladies and gentlemen, Thanks for joining us. I am your host, Michael Fienen.

I’m doing quite well. I’ve got a beard Trim on. Still, I stopped the length, but I took some of the weight out of the sides to surprise the

Well, it it fought. And let me tell you, I may need a new trimmer after that, but I got there. Mm.

Well, woodchuck, hard cider, canned woodchuck, no less. Not the not by the bottle. So it’s I like it this way because Chuck is less. There’s not quite as much here as a consequence of

feel like I’m guzzling bottles of beer, necessarily, but yeah, no, I like it. My wife can drink it. She was gluten free, so this works well for both of us. At that point. As I’ve said before, I don’t really like beer. There are a few that I will have now and then, but generally I don’t like normal beer. So but a woodchuck cider I’m down with and what do you have? Good. Sir,

Um, you’re hooked, aren’t you?

Yeah,

Well, I bought it. I better drink it.

right. There’s a lot of respect for just those good hard workers, you know? Yeah. Little, little blue collar on your scotch,

Nice. You got to find some. I have not seen it around here, so let’s see what we got this. Episode 85 we mentioned this is better development through effective code reviews. We’re gonna be talking about the process of looking at your code, looking at other people’s code. Why you should do it. How you should do it.

Um, And what those benefits kind of our if those words don’t mean anything to you, or you’ve heard the words code review, and you’re not entirely sure what it actually means.

Uh, the definition I stole from over there is an article at Smart Bear about code review that we have in the show notes, they say code review or a peer code review is the act of consciously and systematically convening with one’s fellow programmers to check each other’s code for mistakes has been repeatedly shown to accelerate and streamline the process of software development, like few other processes can. Sorry, practices can,

process of

It’s peer review in the sense of articles, medical articles When, when medical work gets done and published in a peer reviewed journal, the reason that happens is explicitly so that other people will read that chunk of work and then check it right now that process is, of course, a little more drawn out, so to speak. It’s a little slower and longer because their work takes forever.

But in spirit, it’s still very similar to this. It’s just that we tend to turn out a lot of code very quickly, and so that review process just becomes part of the release cycle, as it were.

So let’s start with, uh, explaining to folks why? Just why Code review is good in general. Um, because one of the things that I think can catch people a little bit off guard is this idea that having somebody having to stop what you’re doing to have somebody else look at it and those people are gonna give you feedback that’s going to force you to go back and change work that you’ve already done like that can feel very antagonistic in nature.

It can feel disruptive, disruptive, counterproductive. And so I think it’s let’s kind of start by emphasizing the good of of this process.

So one of the ways I look at this and you know there’s a the concept here that I’m I’m not going to address, like, maybe directly, but I’m gonna try to weave it through some of this obviously code review as a step in your workflow is going to be more ingrained if you work somewhere with multiple people, and that’s part of your team’s process.

If you are a freelance developer, if you are a lone wolf developer, or if you just do it for fun, just because you don’t have yourself surrounded by a team of people doesn’t mean you can’t do code review. You can self review. That’s something we’ll talk about here in a bit, but you can also invite other people.

We’re gonna talk about some of the tools like git hub and whatnot and community coding, and you can invite other people in for just, you know, freelance review. Basically a little harder, certainly. But just because you code by yourself, don’t let that lock you into sort of a myopic view of your work.

Because the reality is there are more than one way more than one way to skin a cat, and it’s going to come down to like. Learning happens best when other people challenge your way of thinking usually. And if you just do things the way you do them, because that’s the only way you ever learned, you’re you’re going to have a design or coding patterns and stuff that they may work. But you may also be using twice the ram.

It may take three times as long as it needs to like there are things that you may not pick up if you aren’t talking to other people about that. So I say I’m not going to address that directly, but I just wanted to say it out loud because it is going to feel like I don’t want people feel like they’re out in the cold. If we’re talking about this and they’re like, Well, I don’t have somebody to come look at my stuff.

Sure, you do ask us. Come jump on our discord and just be like, Hey, guys, you were talking about code review. I’m trying to learn this stuff. Could one of you maybe just look at it and give me some advice? We’ll jump in there. We’ll go look at some stuff for you. As long as you’re not asking us to look at 1000 lines of code,

So and the reason I say that now, specifically my one of my points about why code reviews good has to do with the fact that a code base itself can be incredibly big. Code base can be big enough that you don’t know every part of it. The people at Microsoft who develop Windows. There’s nobody at that company that knows every part of that code. That’s not a thing, you know.

Certainly there are folks who know a lot of it, but they don’t know every little tiny in and out, because you don’t know every nook and cranny that review process can help with things like stopping you from reinventing the wheel. It may shine light on libraries that you have already installed are already available to you that have been built or a P i s that you didn’t know were there but that solve a problem that you’re trying to solve.

Uh, and so that process of review is basically helping you leverage the community collective brain trust of the code base you’re working on and as a result, helps you learn that code base better.

You don’t need to be an expert in the whole code base Obviously, I just said that you can, right? Just knowing. And you learn things. Especially if you’re a junior Devin or an associate developer. You start to learn things about how not just what code is there, but how is it structured? Oh, how would I have gone and found this thing?

Oh, it turns out we’ve got artifact Aries set up, and we’ve got all this stuff in there that I could go look through, um, or search through documentation or whatnot. You start to learn sort of the broader structure and strategy as well, not just the actual axioms and things like that that come into the code itself. Yes, The other thing I’ll say about this is I think it’s really important and really beneficial to review.

Stuff you’re not familiar with is a lot of code. Review is kind of focused on senior members reviewing, you know, mid and and entry level developers work. But there’s a huge amount of value in entry level people looking at the work that the senior people are doing, you know, because they are going to know more and and be more involved in that, and that’s an opportunity to ask questions and to learn again. It’s a learning process.

Review is all about making you better. Um, but when I say that I say it from the aspect of it makes the reviewer and the review, we both better it when that is a functional, healthy process. It should never be one way. It’s it’s not even two way. I wouldn’t even say it’s between those two. Good review is all about the collective the team and making sure, like you said, spread that domain knowledge out.

Get everybody a little seasoned in different things and at least that way so that we can have conversations about, you know, bringing something new in, or how something, you know, change might affect other things. But people have to have some of that experience to be able to do that in my eyes.

Well, the how depends, I think. Right on who you are, where you are, How many people are involved, you know, luckily, there is no one right way to do a code review. Um, before we had fancy tools and like you were saying, you know, in the dark ages when we didn’t have access to something like Git Hub. A common way of doing code review was email like people would literally email code back and forth and use that to monitor threads.

And we’re going to talk about tools that can help you with this. And one of the ones I listed was, Yeah, if you have nothing, you have email and a filter and a template I can give you a system to work with.

Yeah,

So one approach that we see used a lot is and and this is one where right or say like if you’re the lone wolf programmer, self review or a self approval process, where at the very least, you’re kind of taking a moment to sort of step beside yourself a little bit and and spend time with your code, but not as the developer as the person reviewing. Basically, it’s hard. You have to. I think you have to have a good sense of self critique, I think do this. Well,

The way I think about it, too, is like you’re basically playing devil’s advocate against yourself. Um, if you want to think of it that way. Like your role in that instance is to ask yourself the questions, you know? Okay, I did this. Is there a better way? You know, or I saw somebody else can. You know, I saw somebody else do this a certain way. Can I replicate that instead? And why would I do that? Mhm. I agree with you.

I have a habit of So I mentioned last episode. We’re doing this redesign of the drinking you excite. So I’ve got a repo going, got tickets. I’m making issues for everything as I’m going. And I will frequently sort of think in issues, so to speak as a means of, like, brainstorming with myself. All right, stuff down. And then I won’t worry about it for a while. I will come back to it in a day or two when it’s not fresh anymore.

Because then it’s sort of like I’m not dealing with myself in that conversation I’m dealing with past Michael and past Michael knew less than current Michael does so So I can It makes it. Actually, it’s It makes it easier to give myself a you know, a mental disconnect from that original thought so that I can give myself room to question an idea or a decision and think about all the way. Maybe this isn’t the right way to do this, because accessibility concerns or whatever

and and likewise, you know. Hell, I’m I’m sitting there. Fucking code cowboy, committing the master. So good luck.

me at that point.

so talk to me about then. The opposite of that. Like what? What would an approval required process look like?

We we have three. And this is the process we use.

So

not a foolproof process. Mind you, like no,

stuff gets by. Stuff will always be one of the, um, there’s There’s one more, though, that we should hit on first, which has to do with command line merges.

of like a review on demand. Almost.

Let’s let’s talk about regardless of the process you use. I think there are some things to kind of look out for and keep in mind, um, both as the reviewer and the review eat, it just kind of be aware of some of these things because I think they factor into almost any paradigm regardless, um, the first one I’ll mention, because I was just about to say it anyway.

Yes, to be mindful of context, blindness and what I mean by that, because I have this problem with git hub a lot. We have certain chunks of our code base for some files Big 600 lines, 1000 lines. Maybe it’s not great. It would be nice if they were more abstract, and we are working on that. But in the meantime, it takes time to break some of those files down.

And what will happen is if you tweak a function and change a couple variable names, they will show you the little chunk of seven or eight lines that you change and they’ll show you three or four lines above and below that. But they won’t necessarily show you anything around it and

to expand it if you want to see anything. And there are times not every time. But there are certainly times where that context can be very valuable to understanding why decisions get made to do something a certain way. I changed one line of code, but it’s because the tin lines before that we’re doing a certain thing. You know, Um, and if you don’t see those immediately, the my suggestion for that to kind of avoid that is as the developer as the review e.

When you are making the change, that is like that comment on your code when you make your pull request or when you make that commit. And no Tate that line to say exactly that, so that when the reviewer comes in, they know that, you know Oh, hey, I changed this because if you look at the original function, I need to, like, maintain the signature for this reason,

fall off to be,

Well, there’s there’s the phrase we easy

Yeah, the way we say it is, leave it better than you found it Yeah. Yeah. And what we’ll bring up here in this section here in just a second? Um well, here, I’ll just jump right to that. How does that sound? Um, the way we say it at work, leave it better than you found it. This is just this idea of maybe change some bears to const. In your JavaScript. You know, instead of having everything be variable, makes the moment,

Constance, because, you know, they never change. It’s just better semantics in your code at that point. And it’s, you know, it makes the code safer. In that instance, um, you know, little things like that rewrite. And if statement as a turn airy just to make it shorter and more readable, like little things like that, that can be done in the context of your work. This should not be confused with, and and I forget which article it was.

But there’s gonna be an article in the show notes that has this. There’s a joke about things to avoid review. Time is not a developers opportunity. Two suggest that the review we fix all the problems like you review time is not the time to say, you know what? I think this whole thing would be better as a react component instead of part of this angular thing like, No, I’m not gonna take. I was fixing a variable name.

I’m not going to take 12 hours to rewrite this whole thing out of that.

code

and I would argue that if if you need the document unnecessary change like that, then you make an issue. An issue? Yeah, you need to create a ticket for because nobody’s going to go back and look at that review and see that comment and remember. And, um, anyway, so that’s that’s number one, the other one that are the other two. These two kind of go together.

One is that you shouldn’t have to spend a lot of time on reviews like Oh gosh, no, Uh, definitely less than an hour of your work day. Should ever be spent on reviews. Unless you are just a huge software company and that is, and as your QA or something that’s your job, you shouldn’t be committing tons of time. And the way you do that is individual reviews. Nominally should be kept under about 400 lines. That’s based on research that Smart bear did.

Like the thing the review being requested should be under four headlines like In In Total, because beyond that, you start to get a drop off in the number of things caught for the time invested.

I understand that can be hard, especially if you’re working with, like, branches and pull requests. And it may be a lot of individual pieces of work that you’ve done, but like Git, Hub says, Yeah, but we’re making them review the whole PR and the whole PR is 25 files, you know, 14,000 lines of code. I get that, and that’s where it goes back to what I said earlier as the developer comment.

The stuff that’s especially important and if you can try to submit stuff in smaller chunks as you’re going, don’t like. Try to make one big PR out of. There’s no rule that says you have to do that. Make two branches and start your development and one merge it into the other with that one being the one that you will ultimately merge in your staging branch or whatever.

complete

And there will always be exceptions.

Yeah, I talked about context blindness, but you also get a code block blind that’s trying to stare at all that stuff and read through it and see well, did they miss a bracket that they, you know, name their variables, right? And after a while, you don’t care.

2 to 3 minutes. That’s about our average code review or is probably 2 to 3 minutes. And one of my big ones is providing the why behind a review? Because, um, because again,

But I’m just gonna say it. It comes back to this learning opportunity, right? Like if you just say, do this a different way. But you don’t explain why that person doesn’t understand what was wrong with the way they did it, you know? Was it not appropriate for the code styles you guys use? Was it using the wrong AP? I Was it not, You know, you know formatting the tabs, right? Like, what’s the deal?

So you try to explain why something should be done a certain way. There’s an article over Tech Republic. They say, um, never return null when retrieving a list of records from the database say when returning records from the database and they and there aren’t any. We should return an empty list because all the list methods work on an empty list and we don’t have to introduce error handling code.

Mhm. So instead of saying, Don’t do this, you say we shouldn’t do this because we have things to handle it. And now that person knows in the future that that’s the case

you have agency in the solution. And like in this case, if it was something like what this Tech Republic article said, you know, you know, we, uh, shouldn’t return empty list because all the list methods work on an empty list and we don’t have to introduce error handling code like that’s all about the code that has was written before you, probably. But you have ownership over that, too.

Like that’s That’s something that is now yours as much as anything. And you deserve to benefit from that knowledge. And that code

Yeah, Regardless, the lie should be included. Yes, the Y must

and and even to that point just thinking about summaries. It’s also okay to ask for specific feedback, too. Yeah, like, you know, we we’ve talked about this in past episodes and and looking at code and knowing that I wrote code that works. But I would really like for somebody to help me write it better. Like just say, Could could somebody look at these wild loops and and see if there’s a better way?

Because I feel like it’s very heavy and you can draw some extra attention to those because there are a lot of cases and things like that where the reviewer, even if they are, you know, a really talented high level developer they may look at and be like, Yeah, that works. And they may not take the time to show you a better way because they do have stuff to get to.

And so they’re going to be like, You know, it’s not worth explaining at that point. But if you want to learn that, ask

The the next one. I need you to pay particular attention to for me, Erin. Mhm. I need you to not be a jerk.

I mean, I don’t need you to help with the site redesign. If that’s gonna be your attitude saying Uh huh.

the this goes So this goes every way, right? Don’t be a jerk when you write your PRS and in all these articles will have in the show notes. You know, many of them talk about this in different ways. I sort of summed them up under this, but it’s like, understand your tone, understand that text does not convey tone right and know that the people you work with want to make improvements. You know, they are part of the solution.

They are on the same team with you. Uh, they want to learn why suggestions should be considered, you know, regardless. And if you get that feedback, don’t be a jerk about it. And we’ve talked about this before in other context, this idea of you know, sometimes you need to sit down and be a little introspective and learn how to take criticism. And hopefully that is very constructive, positive criticism.

But even when it is, sometimes some folks do have trouble with that. And it’s not about you. It’s It’s not about making you feel bad. It’s not about tearing your code down. It’s just about Hey, we need to do this a little differently because and as long as that because is there, then they’re probably trying to do their best to help you.

So

And I mean, that can be really apparent in some of those power dynamics, right? If it’s an associate developer and a senior developer, there’s a power dynamic in play there. And even if you work at a really flat organization, where yeah, you have titles, but really everybody is doing everything that’s some people

like and that’s kind of how we operate. I mean, we’re a three man team at work, so it’s not, uh, three people team, Um, but that power dynamic, like I’m the senior Deb. But I had no point want any of my other coworkers to feel like I am trying to pull a lever over them, you know, And I expect them to come at me on the stuff I write because Lord knows, every one of us will cut a corner somewhere once in a while when we’re frustrated or agitated or just tired.

And so I expect you to call out my code to, And that goes both ways and so understanding that is really important. And in some organizations, especially really formally structured, one that’s large, those power dynamics can be a little more powerful.

And those people have to understand that you have to realize that just the fact that you’re chiming in can feel a little punchy, so to speak. So you need to kind of nurture those relationships and and be cognizant of that,

point, you know? Yeah, Yeah,

I mentioned keeping recommendations in scope. That’s what I started with. That’s just that idea. Don’t ask them to do things that are way, way out of band for what’s happening. Um, the other part is, and this goes along more with what you were just saying, You know, don’t be rubber stamped, be engaged, right? And that goes both ways. If you are doing code reviews, that is a part of your process.

And as a result, if you’re not engaged with that process, you could be blocking stuff from moving forward. If you aren’t reviewing, then you’re making it harder for everybody else to review what’s there. And if it’s your code, you should be responding. When people leave your reviews, at least tell them if you did it or won’t do it, um, let them know that their review was, you know, picked up on, so to speak. But not every review is going to result in the action.

You know, they may just be saying something to help you know something. We do that all the time, like it’s like, Hey, you know, next time I might do this this way because you know we have some utility classes, but this isn’t like doesn’t rise to the level of going back and unwinding the work. You know it still will work fine and is acceptable. You know,

Yeah, there’s and I don’t like that and I can’t think of what it is. Yeah, I don’t know what to call it. It’s along the lines of like, a leading question. Basically.

Um,

but then

The very last one in this bank is, um this is the hardest one. I think it can be really hard to do this really appropriately, and it depends a lot on your workflow overall. But try, I say, I I say this don’t review at the very end, Really. What I should have said is, Try not to review at the very end, because it goes back to what we started with with this idea of Don’t spend more than an hour doing code reviews.

Try to keep code reviews under 400 lines. You want them to be fast when especially because because what happens is right. If you have a big chunk of work and that review doesn’t come until the very last PR, then that’s where you end up with 247 files. You know, 33,212 lines of code for people look through,

it’s just it’s daunting. And so if you can try to do it as you progress. There is another really good reason for this, though, because if you did mess something up or you did do something wrong or out of pattern and it’s not found until after you’ve done a week of work on it. Ungh, baking that cake can be a pain in the ass. And everybody knows that feeling because we’ve all had that happen at one time or another one time

The classic example of this is Oh, hey, these buttons aren’t accessible, right? You didn’t make any of this stuff. There’s none of the keyboard controls Work on any of this page because you didn’t use the right elements.

Yeah, because I mean I mean, like, you put it on the other, the other metaphor, right? Just pulling a thread on a sweater, right? You’ve You’ve got this whole thing put together and you’ve written the a P I s into it and all this stuff and you realize, Oh, I’ve got to re factor this.

And then that snowballs into every other piece of your code, slowly like, yeah, whether it’s in baking a cake or pulling on a thread, you can cause a lot of damage by not catching something like that early enough and cost a week’s worth of development time as a result.

Yeah, so it’s close out here just to talk about some of the tools and some of what can help you with this process. Um, the first I want you to go check out code mentor dot io over on their blog. They’ve got a checklist. So if you want to build a code review process for your work or something like that and you don’t really know where to start, they’ve got a check list and you can pick and choose from it.

You know, for what’s useful for you, you don’t have to do everything, but they’ve got things like, you know, check for readability, check for maintain ability, check for security speed and performance documentation.

Make sure you know, does so one thing like I’m pushing on right now at work is whenever JavaScript gets touched and you do something with a function or make a new one, it needs Js docks so that we can, you know, automate the production of documentation for that code now. And if somebody new comes in and looks at that code, they understand what it’s doing and why. So make sure documentation is there.

Check for reinventing the wheel reliability, scalability, reusability patterns, test coverage and test quality. You’ve brought that one up, um, fit for purpose. Notice what’s missing. Zoom out That gets back to this idea of looking at context. So this is a really good list of things. You can go look at the little bit longer article because they go into each of those and why.

But, um, I think that’s a good place to start for one, if you just kind of want to understand, you know, and it depends. How formal do you want that review to be? Every review does not have to have a checklist. You know that’s not necessary. If it did, some reviews would take longer than you know, the work that actually produced them in some of those cases. So don’t feel like you have to do it every time, but it can be a starting point

Um, the next thing this gets into a couple of different kinds of tools a little bit, but make sure you’re using winters and a static analysis tool and something like a like, prettier NVs code.

their opinions.

But they also they’re a pain in the ass because they do the things so to make the

like you were saying, right? Oh, a missing semi colon or a missing bracket or something like No, that no longer is a thing like those. And what some of the articles will talk about is anything you can automate. You should. You shouldn’t be relying on somebody else’s eyes to notice that you missed a semi colon in your code. Especially when javascript now doesn’t care. Like

and you can take If you use V s code and you have the prettier plug in installed along with 11 million other people, you should have it if you don’t, um, you can set that when you save a file to run prettier on your code just automatically, and just that’s the way it works. So you never have to think about it then.

there,

other the other tools. I’m gonna just lump these together really, because they’re all the same. Crucible, get hub bit bucket. Um, these are tools. Crucible isn’t at LeSean tool. They’re the folks make the era and confluence and bit bucket, for that matter. But crucible is like a standalone tool, um, that they make really. It’s only gonna be good for enterprise setups, but I include it specifically because of the way it integrates with all of those other tools.

So it’s kind of one of those, like if you’re in the ecosystem, it can be really beneficial because of how how tight in it is. But get hub. We’ve already referred to its free A. You’re probably already doing your code versioning and management and issue tracking and stuff there so that even though the review tools are pretty rudimentary, it’s there. Um, and you can do a lot to kind of beef those up bit bucket has added it recently.

Um, I’ll have a link to the instructions there, but it’s very similar to get Hub at that point, Um, in terms of how they tied it in, um, there are. And for what it’s worth, I know some of you are probably gonna be like, but you didn’t mention this tool or whatever. Yes, there are a lot of code review tools out there. There are a lot of them.

The problem is most of them our apart from everything else, and they tie in the guitar they tie into s V N or they tie into any of these systems, and that’s all well and good. But the reality is, if you’re going to do that, you might as well just use what’s in git Hub. That’s my personal opinion on that, Um, one thing I always worry about is adding yet another tool right to a workflow process.

And it’s like, Yeah, okay, now, you know, now we’ve got this code review tool that’s out there that we have to go log into and check that on top of it. And it’s not in our ticketing system. There’s no tracking. It’s not in git Hub. So I just I say that for that purpose that, yeah, I know. If you wanted to go out there and and grab, grab a review board, grab fabricator, grab code sense. Yes, I know these are all things Garrett road code.

million tools for it, and I’m sure many of them are incredibly good. But I tend to feel like the best tool is the one that’s right in front of you. And since most repose now have that option, you’re in pretty good shape.

this is

I agree, folks, I hope you enjoyed this episode of drunken new X podcast. I hope that you’ve learned something about code review or you’re excited about code review are interested in it. Let us know you know how your experiences go with that or, you know, horror stories. Maybe you’ve had a really bad time with it. I would be interested in hearing that to understand why you know what went wrong in those situations.

And, you know, is there advice we could provide you or, you know, help we could offer? I’m happy to do that. So, um, if you want to share any of those stories or have any questions about that or like I say, do you want to have somebody review some code for you so you can get a feel for it? Let us know we’re on Twitter, Facebook at /drunkenux.

You can get us on instagram.com/drunkenuxpodcast or on our discord at drunkenux.com/discord. I will make Aaron do all the work if you do that. So

If I

Even if you don’t.

how will you ever learn a new language if you don’t sit there and read it? I don’t know how I don’t It wasn’t a joke. I’m just saying, like, you gotta You gotta get in there like I can read java. I’m not gonna write it, but I can read java. Usually I understand it.

I’ll actually give you. Give one last piece of sort of parting advice here, and that’s a if you are a teacher, have your classes review each other’s work. Don’t grade them entirely on your own. Instill in them like use tools that allow your class to collaborate with each other. So even if it’s public, get HUD repos. That’s good enough.

Get in do code reviews and get them used to that process because it’s going to make them better when they get out of school. And it’s going to get them used to getting that feedback and learning how to respond to that feedback.

great idea. I think at the end of the day, it really just drives home this idea that if you want to produce the best code and the best work that through that process, through that good code review, you will inevitably keep your personas close and your users closer. There’s been,

You’re welcome. Bye bye.