Coding Blocks - Google’s Engineering Practices – What to Look for in a Code Review

Episode Date: May 26, 2020

We learn what to look for in a code review while reviewing Google's engineering practices documentation as Michael relates patterns to choo-choos, Joe has a "weird voice", and Allen has a new favorite portion of the show.

You're listening to Coding Blocks, episode 133. Subscribe to us and leave us a review on iTunes, Spotify, Stitcher, and more using your favorite podcast app. And go to http://www.codingblocks.net for show notes, examples, discussion, and more. And feedback questions, rants can be sent to comments at, which is if you hit shift 2, it's that symbol, like the circle with an A in it, codingblocks.net. Hey, by the way, he left off the S and the HTTP because we are secure. I was going to say the same thing. Yeah.

Oh, damn. And that's keyword codingblocks, by the way. Keyword. All right. Keep word. But first, this episode is sponsored by the University of California, Irvine Division of Continuing Education, one of the top 50 nationally ranked universities. UCI offers over 80 certificates and specialized programs designed for working professionals. And Datadog, the cloud scale monitoring and analytics platform for end-to-end visibility into modern applications. So today, we are continuing to talk about the Google engineering practices, and this one specifically related to code reviews.

So this is a continuation of episode 132. We're going to pick up where we left off there. But before we do that, we have a little bit of podcast news. Hey, huge thank you to CodeWithHimanshu, SpaceDuckets, and Akira Kinski. Sorry, on iTunes. Appreciate those views very much. Thank you. All right. And here's Alan playing his joke on me. So one of these I'm going to – oh, boy, this one's going to be rough. I promise. Okay.

So thank you from Stitcher from Anonymous from Croatia. And then Morali Suriar. And then this next one, I'm going to try it. It's going to be something like Leon Farrow Pilgrimage Gil. No. I totally threw in the pilgrimage part. Yeah. Ian Farrow Pil...

No. I don't know. Alan wouldn't let me cheat and look up how to pronounce this one. So, honestly, I don't know how to pronounce this one at all. We will actually have a link to the Wikipedia for the person who left this review. That was the crazy thing. There was a Wikipedia page for this name. That's so awesome.

And I love it that somebody took the time to give a tongue twister. So when I saw that, I was like, well, Outlaw obviously has to read this one, right? Like there's nobody else who should touch it. Okay. But now did you cheat and you already looked at how to pronounce it? I have not looked at it. Okay. Both of you take a turn.

Go ahead, Joe. I'm looking at Wikipedia to see how it's pronounced. Oh, come on. Don't cheat. Oh, how to say it. So it's, it's whalish so i also apologize that's actually not terrible i would have said lamford piggle gwingle so yeah i don't but yours sounded much uh more authentic you know i thought maybe it could also sound much more offensive depending on

who you're asking. So I don't know if you want to go and get to reward him too much for that. I do think that the other one's probably Mirali instead of Mirali. But we could let him weigh in on that.

And speaking of which... What was the difference there? You said Mirali? I said Mirali. Aw. we could let him weigh in on that and speaking of of which was the difference there you said morale i said morale aw ah okay okay at any rate so here's the cool part so we started this episode 132 right and we were talking about the engineering practices and all that well morale actually works at google and he took the time to head over into Slack and drop us some notes on some of the things because we had mentioned the owner's file and I was like, oh, you know, they mentioned it in passing and I don't know much about it. And so he actually gave us some information, which is really cool. So I'll try and keep like a cliff notes of this for talking points right now. But

if you'll go up to the show notes of this particular episode, so codyblocks.net slash episode 133, you'll actually see, like I quoted quite a bit of his conversation because it's some really useful information. But basically what he was saying is every directory in their mono repo has an owner's file, which has a list of people that own that section. And if you were going to get a PR approved, it has to be approved by somebody listed in that file. And the way he said this thing works is very much like, uh, like web configs or any kind of configs and like a.net project or something like that. If there's not one in the directory you're in, then you go up a directory to find the owner's file, right? So if your three directories deep, there's nothing

there. You might have to go up to the root level to find that owner's file. So that's pretty cool. What was owner's approval is something along the lines of, is the CL appropriate for this section of code? You may need multiple owner approvals if you're touching pieces of code that have different owners. So, you know, maybe you're integrating with the library into somebody else's project, something like that. Like there might be things there where you have multiple approvals. And he actually shared a link from the Chromium project, which anybody that uses Chrome, you

know, you can go over there and look and see how they're doing that. Isn't that like the whole internet now? Yeah. Just about. Yep. I mean, because even Microsoft is based on Chromium now. Or I should say Edge is based on, not Microsoft. Edge, right.

Yeah. That'd be weird if it was all of Microsoft based on Chromium. Right. Windows is going away. Yeah. So what did they have? They also had readability. So outside of whether the functionality was there,

this is sort of what we talked about, right? If it doesn't pass based off the style guide and all that kind of stuff, it can absolutely get kicked out, which I thought was an interesting one. You know, like you have a language expert who could weigh in on the pull request. And even if it's functionally right and belongs in that

area, that person could come along and be like, no, this is not very readable per our design standards for this particular language. And it doesn't have to be the owner either is basically what he was saying, right? Like you said, it could be another person that is just truly concerned about how readable the thing is. That's the only skin they have in the game. So that was interesting. And what else did they have? So here was another one. And I'll read this one a little bit because this one will make more sense instead of me trying to paraphrase it.

So he said, so for example, say I'm making a change to some Python code owned by my team. It's my team's code, so I satisfy the owner's requirement. It's production code, so it needs at least one other reviewer. Some teams allow some types of documentation sales to not require review. Also, everyone has their own private experimental directory for non-production stuff they're messing with. You don't need to code review to submit your own experimental directory but it's python code so i need a python readability approval i don't have python readability myself so i'll need a reviewer to get me that so so yeah they have these multiple levels

of what has to go through a pr to get approved before it actually goes into their code base. So this was really cool. And then the last thing is they actually have a case study based on their modern review policies, their code review policies, and they have a doc that's published at research.google. So we'll have a link in the show notes to that. And I mean, again, just amazing stuff.

So Morali, thank you for taking the time to actually write that up. It's awesome to get insight into the kind of things that are happening. I know that we're going over the documents that they provide, but it is really cool to hear somebody saying, yeah, this is what we do. Some inside. Yeah, right. On a day to day basis. So very cool.

And oh, sorry. And, and it almost, oh, sorry. Well, one last thing we've said it, I don't know how many times, if you're not a part of the slight group, like you're missing out, like there are people dropping stuff like this all the time. And it's, it's worth going in there and being a part of the discussion because it's a, it truly is a community of people that are really geared towards helping each other. So I highly recommend checking that out. You can go to coding blocks.net slash slack to, to join up.

All right, go ahead. So do y'all check in your sandboxes? You have like a directory of just kind of stuff you're messing with. I usually do those in separate repos if it's like something. Yeah, I do it both ways. I'm bad about it. Same thing.

Yeah. What, Mike? I'm bad about not doing it. Unless I'm ready to share it with somebody else. If I'm not, then I'm just like, eh, what's the point? I don't care. I worry about it. That's honestly the only reason I do it is what if my computer gets nuked tomorrow, right?

That's really why I'll put it up in like a – I've seen sandbox. I've seen playground directories and stuff like that. I'll do that. Otherwise, I'll do exactly what Joe said and I'll create another repo and just push it up somewhere that people won't – it won't muddy up the main repository. Where it gets to the level of concern that I'm afraid if i lost it then that's also kind of the mindset where i'm like i probably want to share this anyways right yeah fair enough but um i was regarding uh morales no yeah okay yeah uh comment um the uh. The way he described the readability is it almost made it sound like, at least in my mind as I was reading, it's like, oh, it sounds like you have to have like – it's like a badge, you know, like you're gamifying the whole system, right?

And it's like, oh, I have Python readability. I have the C Sharp readability badge. What do you have? You know? It did very much sound like that. Which is – I mean, it's good. I think the thing I like about that is it takes the onus off one single person to have to do everything. Right. Cause I mean, we've gotten pull requests in the past and it's like, man, it's a chore sometimes

to go through it all. Yeah. And I, and I also liked that it was people being assigned looking at the pull request with a specific set of blinders on, like, I don't care about the functionality of the code. I'm only looking at the readability of it. Like how readable is this thing? Right. Or somebody else who's only focused on like, you know, does this even belong in my area? What were you trying to accomplish? Yeah. It's not as overwhelming. And then the last thing I want to point out, because I don't think Joe Zach had actually done this, is he spent like, I don't know, a month and a half nightly going up on YouTube and live streaming him doing some leak code problems. And there's several videos up there and if you've never actually seen Joe Zach in action it's pretty fun because I don't know man just it's worth checking out I have a link to the playlist because I think

Joe you actually did a really good job of putting all these in a single playlist that people can go up and check out so somewhat good yeah so definitely go up there check it out you know it's good fun and you'll learn something more than likely. I've done 60 days now. I haven't streamed the last couple days, though. I borrowed my webcam, and I got a couple days off. I was like, this is kind of nice. I could just do the problems and move on.

It's definitely much easier to just do it without talking. But yeah, I've done the daily problem now for over 60 days. And I'm going to try and hit 100 and just see how it goes. Maybe I'll go loop back and try some of the earlier problems and see how it goes. But yeah, it's kind of cool. Nice. That's amazing. All right.

Who wants to kick us off now? All right. Well, I can take that. So today we will be talking about in more detail what to look for in a code review. So starting with design. And Google says that the most important part of the review is the overall design of the change list. Isn't that crazy? The design.

I mean, that, that, I don't know. Like I, I was thinking about everything that happens and I don't disagree with it, but I don't know. It was just weird that,

that they place that much emphasis on having a good design of what's happening in the code. Yeah. I was thinking like, you know, imagine you get some code to write and you put it in one place and someone's like,

no, there's a better place for this. You need to get it in this other place. Like, fine, you go move it. It means you get a different viewer. You take it to that team. And they're like, no, no, it goes better in the first place. Ah!

Well, see, I guess that's probably the thing that we should point out is when they say design, they are talking about that. Does the code make sense? Is it in the right place or does it belong in a library? By the way, I love that. Does it meld with the rest of the system?

And is it the right time to put it here? So like that's what they talk about design. And I love that because I know I know the three of us have always we've looked at code before and we're like man this should not go here but we have no precedence for for basically slapping somebody's head and being like don't put this here right um or or there's already a cruft of stuff in a particular area and you're like is this the time to draw that line in the sand you know like there there are those time to draw that line in the sand? You know, like there are those questions and I love it that they take a really hard stance on this and no, that's common functionality.

That should be in a library somewhere or, you know, whatever. Like, I don't know. What are your guys' thoughts on this? I loved it. I mean, it makes sense because if it's not already a good design, then that can affect the readability of it, right? Because if you're going through a bunch of hoops trying to make something happen in an area of the code that probably shouldn't be happening anyways, then you could have a bad architectural design as a result. Yeah. Have we talked about before?

I've heard that Google services are typically written in C++. And so even the code for libraries that they publish to other places, a lot of them still publish Java versions of services they use internally, which are C++. And I've heard that the kind of reasons for that are a lot of the services that they bring internally, that that they work on internally will have all this other stuff that only makes sense for them it's like i get you know dependencies on like internal tools or things that are designed for their scale like um like the beam is the stuff

i particularly heard about so if you hear something about like someone talking about something a service in google that's typically written c++ but if they publish a uh you know a package or a library or something for it it's typically something in java because it's just easier to document but it's not actually the same exact code that they use internally you mean when they publish it i'm sorry you mean when they publish it for external yeah huh so they use they'll use a c-sharp version but the version that they'll make available to the rest of us will be a java version yeah so it's like a fork of what they actually use uh yeah the one that they publish so like if there's a you know some beam library or something

that it'll typically be written in java huh that's pretty cool but that's not fair i heard that on a i heard that on a podcast so i don't know how true that is. But I just thought it was kind of interesting to think that, like, you know, they'll say, like, if someone just says, like, whatever service, it's probably C++. And let's say, say, like, the Java service. All right. So next up we have the functionality.

So we talked about this on the last episode. It's funny that this is even a bullet point to me. Does the change list do what it's supposed to do? Okay. I would hope so. But in fairness, I have looked at pull requests before and said, wait a second, I thought you were trying to solve A, but it's not in here. So it's weird that this is in here, but there's been a number of times, at least in the reviews that I've done where I'm like, you're not solving the problem. Like there's a lot of code here, but I don't see what this is. You're creating a whole new set of problems. Yeah, no, I mean, you're right. It's weird, but rightfully so it belongs here.

Yeah. So, and I think, sorry, but I, you're right. It's weird, but rightfully so. It belongs here. Yeah, so. And I think, sorry, but I think that that's just, it's no fault, like, you know, any developer who might be guilty of this, right? It's all coming from a good place. Like, you see something, you're like, oh, I should change XYZ while I'm there. Or, oh, I can improve this system by if I add this other thing in. And so that's where you can kind of get like this scope creep. And, you know, by the time you're done, you might not have even solved the original problem. And maybe it's just because you're like, hey, let me go ahead and commit this thing in, get this into use sooner rather than later.

So it's not necessarily a bad thing, you know, or coming from a bad place. But yeah, that's, that's tougher though, because I would think that even for the change list to go in, you kind of have to define what your attempt, what you were attempting to do. So I guess in that case though, I would think, you know, the main thing was to add widget A, right? And instead of that, you ended up doing a bunch of other stuff. It seems like you wouldn't even include, hey, I added widget A. You just say, I did all this other stuff. So it's weird.

I guess in the story that I was thinking of in my mind is that somebody was like, okay, Hey, I'm supposed to add widget a, but as I'm iterating towards that here, I've added a new dependency injection framework. And I'm going to go ahead and commit that and let that get reviewed and other people use it because it's going to help me out later. So here's the change list for ticket add widget a, but yet I'm not actually adding widget a yet i'm just you know entering my way towards it yeah i could totally see that so one of the other things that they ended up pointing out here that i thought was interesting is not only does it do what it's supposed to do but is it a good change for the users and the user could be either the developers that come behind you or it could be the actual end users of the system. And I like that a lot too, right? Like, Hey, don't just put it in there because,

Hey, they marked something done. It's Hey, is this an actual improvement for everyone involved? It's so easy for us to just focus on what's easiest for us to, you know what I mean? Like always just focus on like, Oh, what's going to be better for the next developer to come along. And yet, you know, somebody else will come at it from like a higher picture and be like, the user isn't going to care about any of this. What are you doing? That's a good point. Oops. As a reviewer, I do like this one too.

And this is, by the way, why outlaws included on most pull requests that are ridiculous is as a reviewer, you should be thinking about all the edge cases, currency issues, and generally just trying to see if bugs arise, looking at the code. So not necessarily going and running it, but are there any glaring holes just when you're just, you know,

skimming through it and looking at it and seeing if it makes sense. So it's one part turning on your human compiler uh-huh and two part turning on your human test runner yep that's really what it is right and warnings warnings as errors too warnings as errors uh it's hard to look at those that's hard to do though to like actually read through the code it's hard enough to just just read through something and see if there's compilation problems, which that's the most frustrating thing. right away, then you can bet the rest of that code review. I'm going to be like really particular about every little thing, right? Like, I don't know. And that's probably not fair of me. No, it is fair. It's totally fair. If you're going to waste my time that I'm about to put some hurt on you, right? Like that's, I think that's totally fair, but I think to back up,

this is where having a good build pipeline in the first place can help you out, right? Because that thing should fail the build and you shouldn't even have to look at it, right? Like if you open up a change list, as they call it, and you see that it failed some builds, then you can just write back and say, I'm not looking at this thing. It broke over here. Go figure that out. Well, that's assuming that you have a pull request, you know, like build validation type of rules on your pull request or your change list as Google refers to them. Right. So, and I mean, and we've definitely been in environments in the past where we didn't have that. And so it would be easy for somebody to just like write something and just assume it would work and try to sneak in a pull request.

And you look at it and you're like, hey, wait a minute. And it can be something simple like, hey, you forgot a semicolon. There are tons of people that think, oh, I can just go make this change and push it in. And I've even been there, right? Like I'll think, you know what, this is a simple thing. And you should never do that. You should hit the build right after you make your change just to make sure. Right.

So let's see what else they have. As a reviewer, you can verify the change list if you'd like, or you can have the developer walk through the changes, the actual implemented changes. So not just going through code, but like, let's say, for instance, if you're working on something that's UI related, it might be hard for you looking at the code to visualize what's supposed to happen, right? Like there might be a series of steps you have to do to get this thing to trigger, do whatever. So you might actually get a developer to walk you through that in the UI. So you don't have to pull it down, compile it, deploy it, all that kind of stuff. Right. And I think that's fair. What if you are cherry picking from one branch to another do you build and test every time i know outlaws answer

yeah i think i can guess your answer i think it depends on where that cherry picking is happening if you're using something like visual studio online where you can auto cherry pick into new branches. I don't think I test it as thoroughly there. Whereas if it tells me there's a conflict, because usually it'll tell you there's a conflict and you're going to have to do it locally. If I have to do that locally, I do build it and test it out locally. Yeah, definitely conflict. But yeah, I mean, if it's just like a real simple like, oh, just copy these couple lines there, like what could go wrong? Then it's just like a real simple like oh just copy these

couple lines there like what could go wrong then it's just so easy to not rebuild it but it's possible for things to mess up because you know something later in the line or so i changed a variable up higher where you can't see or something like that so it's dangerous to do that but it's it is yeah exactly if the tools didn't make it so easy to push a button and make it happen, then I think that I'd be more apt to test it. Because I used to do all the cherry picks locally and push them up. But then when they added that button, it was like cherry pick and merge. Okay, cool. So I'm probably going to disappoint you here.

So for me, I'll take the time on that first one right but then when it comes to the cherry pick assuming that there are no conflicts then i will look over what the change list is through you know like the the diffing tool and just verify that it looks about like the same and so we're like you know it didn't just randomly insert an if block into like what a string is in some new file or whatever, you know, or what some new change version of that file. You know what I mean? If I see that like things still look to be in about the same spots, then I'm not going to bother to build the the cherry pick version. I'll let the I'll let the build tool do that for me and verify, you know, because I've already done it for the original chain set. So I'll let the build tool do it for the build validation for the cherry pick as well as running any test for it for that cherry pick.

But I didn't visually inspect it. For anybody that thinks we're crazy talking about cherry picks, if you have not listened to the show where we talk about comparing various get workflows it's highly useful if if you've only ever done things a particular way i don't know the episode number but the name of the episode is actually comparing get workflows so um yeah cherry picking could be your friend if you have multiple concurrent releases, then it's definitely friendly. Yeah, it solved a lot of pains for us. It was episode 90. And as you might be able to guess, it's one of my favorite episodes. Of course it is. All right. And then the last thing that they say here is when they're doing these changeless reviews, they specifically call out parallel programming types of issues because they're hard to reason about. Even when you're doing these changeless reviews, they specifically call out parallel programming types of issues because they're hard to reason about.

Even when you're debugging, if you don't live in that world where you're working in concurrent type or parallel type things, just even understand the debug output that you get from that stuff can be difficult. So when it comes to those types of things, they truly want somebody that's more specialized looking at that so that they can identify where deadlocks might happen or those types of things. Yeah. It's always so much easier to just spin up a thousand instances of your

application rather than have your application spin up a thousand threads. And then you try to reason about that in the code. Parallel and concurrent programming can be very, very hard. It actually takes a little bit of time to learn your tools to know what they're showing you and what that represents. Has parallel programming kind of gone out of fashion

because of the ability to just spin up multiple nodes? I don't think so. Because threads are still way more efficient than launching an entire thing, right? Yeah. Yeah, I wouldn't be surprised if some people do go about it that way because things like Kubernetes make orchestrating that stuff so much easier. But I don't think that they're... If your problem allows you to spin up a thousand instances of your app to solve the problem, then I would tend to say, hey, just take the easy side here. Because like you said, for example, if it is going to be in a Kubernetes world, it's easy to just say, hey, scale deploy and boom, a thousand replicas, right? But that's not to say that every problem can be solved that

way because, you know, there are other things where like you might, that might require some state that, you know, you have to have some kind of thread manager that's knowing like, okay, you know, your, this thread is particularly for this set of things. And this other thread is for this other set, you know, and you know, your, your problem domain might not necessarily lend itself well to multiple instances of the same application versus just single threads within it. So here's an example. And I remember doing this several years ago. There was an application we had at a company that we worked for that would basically create different size of images, right? For thumbnails, for medium, for, you know, high resolution images. And they had a process that would run that basically as new images got

uploaded, that you would upload the highest res image. And then it would try and create the other ones based off that, right? Well, initially there was a program that basically just looped through and did everything. And it took over a day to finish up stuff. Right. And it was, it was kind of crazy. So what I ended up working on was something that would parallelize that thing because you didn't need it to happen sequentially. You just needed it all to happen. Right. And you had to keep track of what was done. So spinning up multiple threads made a lot of sense and took something that took over a day down to, you know, 30 minutes, right? But here's where that would sort of fall apart.

Doing that in a bunch of applications, single-threaded applications, wouldn't make sense, right? You're going to spin up a million of those things as opposed to, hey, I have this one app. It knows the stuff it needs to work through. Spin up the threads that you need to do, work on them and go away, right? So they're lightweight processes as opposed to full-blown application logic. So I think, you know,

depending on what you're trying to do, it wouldn't make sense to spin up a single individual, you know, hey, I have an image, go run an app to process that one image unless it was just hyper lightweight. But there's a lot of overhead and something even like kubernetes we said right that's a lot of orchestration that have to do so i think that's a situation where you would lean towards parallelizing and doing

thread development as opposed to you know just blocking out an app i don't know yeah i was thinking if you had like um i don't know 20 of a thing that you needed to do then then uh multi-threading is great if you have thousands not so great like if your number of pictures went from millions to billions then you're back to probably looking at distributing the problem you know from computer's perspective because in that case you can just add more nodes and use like a map reduce type framework in order to like do them all in the same amount of time whatever but yeah if you can just divide the problem by 5 or 10 or whatever, some smaller number, and get it to complete faster, then it's just kind of multi-threading is like this in-between now

because it has gotten so easier because the tools have gotten so good for multi-computing, distributed computing. So it's kind of interesting. Yeah, I mean, that's a fair point. It definitely matters on the number of things that you would possibly want to spawn. Because if you try to get into thousands of threads, then you end up taking a performance hit on the overhead related to the thread management for that thing versus if you had a thousand instances running individually, you know what that would mean. So like all our other answers on the show. It depends.

It depends. Yeah. Come here for all your definitive answers. That's right. You know what answer you'll get. That's awesome. All right, so next up we got is complexity. So I also like this one.

They say this should be checked at every level of the change, whether it's single lines of codes, functions, classes, like everywhere. They don't want overly complex code. I question, like, think about how, because complexity can mean multiple things, right? And so to their point about like the single lines of code, think about if you've ever seen like some Perl, for example, right? And you question it. You're like, wait, what is that? It's one line of code, but what is it doing?

Because it's like taking advantage of all these like hidden variables that are, you know, are not really hidden, but built in variables that, you know, you don't know about unless you've got gobs of Perl experience. But if you're looking at it the first time, you might be like, well, wait a minute, where did this underscore dollar come from? Or something, I don't know. It's been a long time since I've touched Perl. But the point is that complexity, you could have a single line. They could have a lot of hidden complexity in it that unless you really know it, you don't know what's going on, unless you're an expert in in Pearl, for example, but that's also where that readability comes back into play

because the readability expert for Pearl might be like, no, this is fine. Or maybe it's not. I think that's domain knowledge. Right. And that's, that's a tougher one. I, I think where you draw the line is, so what you said about like Python having a specific things like I've, I've really started getting into Kotlin lately. And there's also things there that you look at, and if you haven't gone and learned the Kotlin language, it just will not make sense. So there's a difference between understanding the language and the things built into the language versus somebody trying to get clever with a one line link statement that has 50 chains on it. You know what I mean?

And I think that's where the difference is. If you're using built in language support that has that stuff, that's not complexity. That's utilizing language to make yourself and the code more efficient. There's the other of, Hey, I could take this problem that should be on five lines and turn it into a one liner. And that's, that's a, an experience type of thing,

I think. Well, the method chaining problem that you talked about in length though, that would already break like like uncle bob uh you know clean code type of design i forget what exactly he called that one um i think it had like a choo train in the in the book and yes it's choo choo but like an example so some of the things that i ran into in kotlin recently is there's something called reify. And you'll see that sprinkled throughout the code.

And you're like, wait, what is this reify garbage, right? And it's a way for you to actually access the type of a variable, like as a class. And you see these little keywords sprinkled throughout and no clue, right? I'm staring at it going, what magic are they clue, right? I'm staring at it going, um, what magic are they doing? Right. And that, that's not fair because I wasn't a, I wasn't familiar with the language or using like the dynamic keyword and C sharp or something like that. Right. Somebody that has experience in that language can look at it and go, okay, I know what's happening here. So I think what they were

talking about is when, when people are just trying to make things, they're either trying to be clever or they're, they even call it out specifically over engineering something, right? Like we've all, we've all laughed about this before, right? You have something that you need to do and it needs to take steps X,

Y, and Z. And then somebody is like, Oh, let's have a rules engine. You're like, wait a second. we have wood use case like why implement a rules engine right now which is going to take weeks when when we can just say hey after x is done do y after y is done do z so an if statement right right yeah i've seen people reinvent stuff they're like i don't need

to use library for this because all i need to do is check that it has an at symbol and you know dot com at the end or whatever oh that's not how you do an email yeah don't don't try to write your own email address validation you will fail yeah and don't do the regex either you'll fail because the first 20 answers you'll get on Stack Overflow will all be incorrect. Like there's so much. Well, yeah, we've talked about this before and I'm trying to remember what it was. There was a, there was something that I think it's email regex.com that has like, here's the regular expression that'll work 99.99% of the time. And it has like the

regular expression for each individual language. And like the one for Pearl was like 50 lines long or something like that, you know, and it's just insane. And, you know, not to pick on Pearl, but I forgot, like, it's been so long since I looked at Pearl, like all the special variables that are in there. There's like probably 50 of them that that if you don't touch pearl on a daily basis you will not know these things and you're like wait where did dollar exclamation point come from what what where did that value get assigned you know like and i think i said underscore dollar a minute ago i forgot that they're all dollar first. Yeah, not don't want to go back there. Um, one other thing to point out here on the complexity is they said

too complex means it's just not easy to understand looking at it, right? Like it's a simple, it's very simple definition. And what they say about that is if you can't understand it, looking at it as somebody familiar with whatever language you're reviewing, then that means the chances are the next developer that comes behind you also won't understand it. And the chances of introducing bugs because of misunderstanding that code is increased. And I think that's a great point. Yeah. Any bit of code that you don't understand, you're definitely going to introduce a bug if you start messing with it. Can we just say that? That should be a definitive. If you don't understand it and you start modifying, you're absolutely going to introduce a bug, period. Unless, unless you have this next section here, which I'll let Michael take. Okay, fine. So the next section would be test. So usually tests should be added in the same change list, unless this is an emergency situation, which we covered

the emergency situations as defined by Google last time. But I was actually going to say, though, that the complex portions of your code, the more you can break those down into tests, then the more it can be easier to grok just a specific part and then kind of like chain that knowledge together and be like, okay, this is overall what this big thing is trying to do. Yep. You know, this next part of the test is actually funny to me. Make sure the tests are correct and useful, man. I can't tell you how many times I've seen a unit test and I'm like, why was this necessary?

Like what? I mean, you're not really adding anything other than the fact that you bumped up your coverage percentage. Like you didn't add anything useful here. Was it my code? No, it wasn't yours. I don't think.

Well, cause, cause I say that because I, there have definitely been times where like, I guess in my zeal to, to have, you know, good test coverage and to make sure that like, I've thought through all the different scenarios and everything. There have definitely been times where like, after the fact, I'll go back and look at a, at a test, a unit test. I'm like, Hey, wait a minute.

Am I really testing my code or am I testing the result of some library? Like after the fact, I'll go back and look at a, at a test, a unit test. I'm like, Hey, wait a minute. Am I really testing my code or am I testing the result of some library? Right. In which case I don't care. I don't want to test the library. Although, uh, you know, we have definitely talked about doing that too. Um, while covering, uh, the clean code series. But, but I think if you were going to do that, if you're going to have tests for testing the library,

then it should be in a specifically named type of project so that it's clear what you're doing. And in my case, I definitely wasn't doing that. Yeah. You do test code some of the worst code usually because you don't do any sort of abstractions. It's very typical to have a bunch of boilerplate lines that set up a few things, and then it's tempting to just copy and paste it,

because you don't want to go and create a bunch of classes just for drying up your test code. So maybe you've got something that sets up this object, maybe adds a couple test values to an array or something, and then you test with it. And so it's really tempting to copy and paste that test and just change a couple numbers so i don't know if you've seen that where like you've anytime you're dealing with arrays or collections or hash tables or anything where you're doing big initialization either you have a helper function that like creates object or you just have a bunch of code you know repeated so i can't tell you how many times like lead code stuff where i'll copy the test before me kind of change a few things get something wrong like miss a test value or something and spend time debugging try to figure out why something

doesn't work only to realize that oh I didn't change the result value in the expected or you know whatever so the the code worked the test was wrong and that's where copy and paste will kill you every time right like I do it all the time I'll copy and paste something because I'm like I was mostly the same and especially in, and then you'll miss the one variable that you needed to tweak. Yeah. I mean, it's inevitable. One of the things that I like here is they say, will the tests actually fail if the code is broken? That's actually the importance of unit tests, right? Like that is, that's almost the entire purpose of what they're designed to do is to help you as you modify code in the future to make sure that the stuff's still working.

But if it won't actually break, would you modify the code? Are they adding the value that you thought they were? They could be a false positive, right? This is a slippery slope though. So, I mean, we're not talking about the code not compiling. We're talking about a use case that could slip through the cracks in the code that maybe you don't have a test that covers that particular scenario. Possible. And so while we all strive to do that, it definitely happens. And, like, I mean, we've talked about this in the past where like, um, you know, you get a ticket for something. And the first thing you do is you go and create a, uh, a unit

test that's specific to that test case or, you know, to that ticket to try to emulate that test case and then see like, okay, why is this a problem? Why is it not already covered? And what you might find is, you know, um, maybe, maybe you had one that would, that would check your inputs, for example, you know, to make sure that they were like numerics, but you didn't check the boundaries, you know? I mean, maybe, you know, I don't know. You see what I'm saying though? Like, like if you, if you were taking strings in as an input and you're like, okay, let me make sure this string is actually on numerical digits and it is okay. Great. So, you know, and then somebody adds in a comma and now it's no longer all numerical digits or

something. Right. But the point, but I mean, getting back to the other point, edge cases, sure. Right. Like we've talked about this in the past. If you don't have the edge case in there, then when you come across it, then you add that as a unit test, right? And then that way in the future, that one's covered as well. But the bigger thing is if you're using a method that's being unit tested and somebody goes and changes that method and it didn't actually break the unit test the way it should have, and maybe it wasn't an edge test, maybe it just wasn't a good test in the first place, you know, that's more of what we're talking about here. Right.

Cause, cause edge cases are going to sneak in, in many cases, but I don't know. This one's a tougher one. And it's also really hard to identify until you hit some of these things that, you know, will, will trigger those things. But, you know, I guess my point was that like, it's totally fair if it, you know, like in your case, if it wasn't a good test to begin with. But if – I wouldn't necessarily – like you're not – even as a reviewer, you're not necessarily going to be able to come up with every edge case. And until – you can't say like, well, until I've decided every edge case and have decided whether or not i think that the the tests you've provided will cover it you know whether or not i'm going to approve it or not like you know what i'm saying does that make sense yeah it does you can't let that be the what's going to stop it necessarily and let you know if there's an obvious one sure fine point that out

like hey you didn't you didn't bother to check your range boundaries. The next one we have up here is, are the assertions simple and useful? And honestly, no lie, when we did the unit testing episode way back when, the way the outlaw lays out his test, I've basically gone by that. I've actually gone back to that particular show notes page several times to look at over the years. And that's how I do, right? I try to make sure there's one assert per test, and then I'm testing very specific things. And I can't, I've seen lots of unit tests where there'll be 20 assertions in the same method. And it's like, well, what are you actually testing? Right? Like this, this isn't, this isn't concise, right? This isn't actually testing the one thing that you should be testing.

So simple and useful, I think is, is very strong terms that should be followed. Um, and are they, are they separated appropriately? That goes back to the single assertion type thing, right? Like you should be testing a thing in each one, not 20. Less than 20 is good. You've been quiet. You want to take the next one?

Yeah, naming. So this is really important. We're good names chosen. I am particularly fond of this one. I dislike – there are some names I definitely dislike. But one thing that's really tempting to do, especially like languages java where you have like person person equals new person uh i like to really look to see like well what's the significance of this person like what if if somebody like say for example is doing a pull request for you or is looking at someone's code and they don't know

what use case you're tackling like it'd be nice to know if this person is a customer or uh is this a you know a potential fraudster or whatever is there anything you can do to give some context with your variable name? What's the meaning of this thing instead of just naming after the object? Will you fail? Will you fail a PR based on that? No. Probably not. If it was person instead of customer, if it could have been better, no.

Maybe if the PR was so good that that was, like, the only thing I could think of, you know, then maybe. But if I did see, like, if I see, like, person one, person two, if I start seeing, like, three or four, uh-uh. Oh, there was some code I remember seeing a couple years ago. It was, like, if the C1 equals the P1 dot character and the V3 dot whatever, and yeah, it was just – it drove me crazy. Because, yeah, if you just zoomed in to look at a PR to see what they're changing and you're like, that's the only context you get. And you have to scroll up 100 lines to see that C is customer and V is the value and do that translation and keep that back in your head. There's something wrong with that. You should be able to see what's happening just from those

lines, I hope. If you're going to have variable names that are just single characters, then your entire method better only be like three lines. I agree. Completely agree. What about iterators though? Iterators get a pass. If's an eye you know we're seeing

customers but i typically don't have lambdas that like go more than like three lines or so right and that's probably fair uh what about uh long names i i prefer longer over shorter for readability, but you know, I don't want to go on off side of the screen either. Yeah. Uh, usually I kind of think of like if the method name is too long, it's generally cause it's doing too much. So I think you should name things accurately,

but there's been times where I'm like, well, this method, what it really does is like get student ID and create it. If it doesn't exist, it's like, Ooh, what's a better name?

But that's a common pattern, you know, like create if it doesn't exist. Upsert student ID. Yeah, there you go. Yeah, that's better. So it's short. Because if you see a long name like is this or that, it's like, ooh, you know, that's gross. It's like at that point you're just saying what the code is. You might as well just do the thing.

So back to the question I threw your way. Would you a a change list or a pull request on some of these things hey you say no it depends it depends for me how egregious it is right like if there's a method name that got introduced that doesn't tell me anything about what that method does i'll be like you need to rename that. Only because it's, I think about it from the perspective of who's the next developer to come in here and how mentally challenging is it going to be to backtrack and find where this stuff actually impacts, right? So it's, I'm not as a stickler on internal variable names like inside a method as much as I am for class properties, method names, that kind of stuff. Those are usually way more important to me.

Well, I mean, when it comes to method names, though, we're not – we should, at least in my opinion, caveat this and say we are not including unit test names. Because my unit test names are like off the screen long. Yeah, they are. Um, no, no, we're talking about regular classes,

right? So, all right. How long you want to take the next one? Sure. Although I'm not crazy about it. Uh, comments were the comments clear and understandable.

And so the reason why I say I'm not crazy about it, I think we might've mentioned this in the previous episode, but you know, if we're talking about comments that are like summary documentation type comments, then I'm okay with it. But like, I, I tend to frown upon comments that have to be in like the body of a method, because then as soon as I want to write one, I think to myself, like, if it's that complex that I have to write this out as a comment, then I've already done something wrong. And maybe I should just have a method name that's expressive enough to, to, uh, convey what, what the comment was going to convey anyways. So, so comments, but you know,

asterisk. So it's interesting you say that. So two things I want to clarify here. First off in the Google doc, and I think we lost outlaw a second ago in the Google doc, it says, are the comments clear and understandable in English, right? Like, so that was kind of interesting to me that they called it out specifically for English. And I guess because it's the most common denominator for most of their coders, right?

I would imagine if you're somebody that, that lives in the EU somewhere, and that is your, your programming base, you know, it's going to be whatever language is common for whatever your application is. But I guess at Google,

there's is English. Um, you know, um, I like watching, uh, coding on uh youtube and uh he kind of speaks different languages sometimes but a lot of times he'll speak in arabic if the code is still in english so it's it's funny to be able to follow along and like certain words you know kind of crossover or whatever are common to both so it's uh it's been really interesting to kind of watch that and just see how much you can pick up because he uses good variable names and because the code is very clear. And what's amazing to me, he's a great coder.

You guys should go follow him. He does programming challenges in C++, which like a blush makes me think like, oh, my God, that's terrible. Like it's so verbose and so weird. But like when you see the algorithms, the actual code part of C++, it's actually really nice and terse. It's comparable just a few lines of JavaScript code, but it's not until you get into the header files and the objects and that sort of thing where it really gets verbose. So seeing his code has been kind of eye-opening to me from that perspective to see just how clear and good it is. But it's because he's very terse. He's got good variable names and he's just a great coder.

That's awesome. So following up on what Outlaw said, he doesn't like the comments. This is actually where Google drew the line. They said, if you have a comment in your code, it should explain why the code's there, not what it's doing. So to his point a second ago like going to go about, well, maybe I should name my method better. Maybe I should do this. Like that's all the, what it's doing part. If I've definitely written some comments in the past where it's like, uh, I'm

putting this here because I don't know how you get here in the code. Right? Like I've, I've never seen this happen, but apparently it can. That's the kind of comment that is actually helpful, right? Like it gives somebody an idea of, you know, what was this harebrained thing they threw in here, right? Okay, well, this is why, right? There was some sort of edge thing that, you know, has to be traced back. So personally, I think that's really nice, right? Why not what? And that's beautiful. They said if it's not clear enough on its own, it should be refactored. Which is kind of along the lines

of what I was saying a moment ago. Exceptions. So, there are some things that really do well with comments like Reggex is a big one where pretty much any regular expression should probably have some sort of comment because it's not always clear or at least a really great variable name that expresses the intent there i don't want to see regex r equals and then you know like top hat dollar sign slash slash dot star slash dot like uh no no thanks uh also if it's a really complex algorithm like if you're doing some sort of bfs traversal to you know look at this and you know add these

numbers together whatever that can be really hard to see from the code but can be expressed easily in a comment says like bfs traversal that sums the neighbors or you know something like that it's a one-liner you don't have to like parse through and take notes on what is actually happening i don't know if you ever seen like where someone will post a code for some sort of algorithm and say like what do you think this code does like if you saw the code for merge sort without you know like a good name saying what it does like it's really hard to see what the heck it's doing. Yeah, I like that. Yeah, I did forget about regular expressions because that is definitely one where I've actually commented in the past where like that's the one area where I do provide a comment on it. Because and I'll even I've joked about in the past where like if you see any reference to monkeys in the in the comment, then you'll know that that was mine. Because I'll write something along the lines of,

it'll say something like, because RegEx looks like it was written by monkeys banging on keyboards, and then I'll have a comment as to what each part of the regular expression are trying to do for complex regular expressions, not for very simple ones. But fine. So you discriminate against people that don't have a deep understanding of RegEx, is what you just said. Uh,

no, I don't think that's what I said. No, no. What's this dot star thing? I mean, have you ever heard that? What was that?

That, uh, the saying that was like, you know, if you, if you gave, uh, uh,

an infinite, an infinite number of monkeys, infinite number of time that if they were just bang on keyboards, they would reproduce every Shakespearean work and whatever. Yep. That's good stuff. All right. And then the last thing, which Mike also touched on here, was comments are different than the documentation of code, right? So when they're talking about comments and code, they're talking about the stuff that's in line that's, you know, hey, why is this doing this? We're not talking

about the stuff that would be spit out in a Java doc or a J doc or any of that kind of stuff, right? That's totally separate. That is there to express the purpose of what's happening, the usage and the behavior of that code. That's completely different than these other types of comments that we're talking about that you'd sprinkle in a change list. This episode is sponsored by the University of California, Irvine Division of Continuing Education. UCI is no stranger to online education, having offered online courses to students around the world for almost 20 years. And let's face it, there's never been a better time to get a quality and convenient online education than right now. Yeah, you can learn from anywhere, anytime by choosing a schedule that meets your needs. Are you looking to get a job in data science? Listen, with UCI certificates in data science, predictive analytics, and machine learning,

students will gain the necessary skills to land their dream data science job. Hey, and if you're looking to become competitive in the global marketplace, advance your career, or start a new one, UCI has the resources to support you on your new path. Enrollment is now open for the summer quarter, with courses beginning as early as June 22nd. Are you concerned about the cost? Well, don't be.

UCI has scholarship options for those that qualify. So learn more by visiting ce.uci.edu slash codingblocks to learn more and reserve your seat. Again, that's ce.uci.edu slash codingblocks and reserve your seat today. All right. Hey guys, uh, Joe's back here again,

uh, doing another, another excellent pitch for reviews because as you know, it's very important for us and it helps us find new listeners. And also we just really like it. So if you could, uh, leave a review,

that would be super good. I don't know why I'm using my weird voice on this. Uh, but, uh, yeah, if you hear this, I hope you consider it. If you go to gun box.net slash review, leave a review. That would be super good. I don't know why I'm using my weird voice on this. But yeah, if you hear this, I hope you consider it. If you go to gunbox.net slash review, we try to make it easy for you. Yeah, that'd be super great. Do you ever think that what's weird about it is that you call it your weird voice? Otherwise, it would have been perfectly fine. I've got a different name for it, but you know. You know.

You know. You know. It sounds more like your smooth jazz voice, you know. That's right. You're listening to the smooth sounds of WJJAZ. You know, there was one stream where I was like listening to this. I've been doing royalty-free music in the background, and there was like this weird, like, I don't know, kind of music. So I did the whole, like you're listening to joe zach on and then uh later when i kind of previewed the stream i realized the

audio wasn't going to be through so it was just me being weird for five minutes that's awesome we're gonna need a link to that one all right i'll try to find that but i mean if you if you watch like they're all kind of weird so you know all right well with that we will head into my favorite portion of the show survey says all right um so a few episodes back we asked what is your preferred method to increase your productivity? And your choices were code to music in the background. Is that freedom rock? We'll turn it up, man. Did you guys go back and look at the commercial that I added for that?

Oh my God. So, so trippy to see that again. So definitely check out. This is from episode 129 and I included a link to YouTube to that commercial. All right. That was like one of those 80s compilations. Sorry, that was one of those 80s compilations, right? Yes. Yes.

Yeah. Awesome. Yeah. And it's, it's, it was, I, the commercial was just around so much that you just heard it all the time and it became this like pop cultural reference, but it really wasn't a good commercial to begin with. It's like, really? That got by? They got away with that? Okay. Uh, your next choice is absolute silence, except for that annoying buzz coming from my UPS and my monitor and my LEDs. Or Netflix encode. That's a thing, right? Or treadmill or bike desk tone my brain and my glutes. Or Pomodoro. I live my life one quarter hour at a time or pajamas all day long.

And your last choice, take a break, go for a walk, eat lunch, anything to reboot. Okay. Uh, I think math wizard has gone first the last couple of times.

So I'm going to give Alan the opportunity to go first this time. All right. So seeing as we have multiple choice here, anybody that gets 50% wins, right? I think we're good. Oh, I forgot. Was this a multiple choice one? No, I don't think it was. I'm going to double check.

So I really don't know on this one. I think that it's going to be between either Code to Music in the Background or Take a Break. I am going to go with Take a Break, Go for a Walk, Eat Lunch, Anything to Reboot, and I'll say 22%. Although I don't think that's the answer. Take a Break, 22%, and this was a multiple choice one. I forgot about that, so thanks for calling that out. although I don't think take a break 22% in this was a multiple choice when I forgot about that. So thanks for calling that out.

Okay. So then take a break and code to music in the background. We'll go combined to aggregate 30%. I don't know what to make of that. Um, so I'm going to do the same thing. Wait, you can't have my number.

Why not? You have can't have my number. Why not? You have to have your own number. You can steal my idea, but you got to have your own number. Okay. 30.0. The math wizard wins again. That's right. Math strikes again.

Well, it depends because see, I'm using a string operation over here to compare the numbers. So, you know, 3, 0 and 3, 0, 0 aren't the same. Okay. So I missed it. So you're going to take a break and code to music, but somehow you want me to equate that into 30%? Yeah, yeah. Yeah, aggregate. Add those,

those percentages together for me, sir. Okay. We're trying to make things easy on you. Right? Yeah. It's not working. Um,

okay. Actually. So because this is multiple choice, what was the one chosen the most? That's what we should do, right? I want to say code of music in the background choice what was the one chosen the most that's what we should do right i want to say code of music in the background that was the highest okay at what percent 30 30 still 30 oh 30.0 that's good for my my string interpreter my string comparison here what about you joe i'm sticking with your original answer.

Okay. So Joe picked Pomodoro at 30.0%. Yeah, sure. No, I'm just kidding. No, seriously, what was yours? I didn't hear it. By the way, I'm having the worst internet experience over here, all thanks to the internet lords at at&t i don't know what is happening so i'm hearing like every other word from you guys and right now you're frozen oh wow that's he's coming through fine for us though that's interesting yeah everything

sounds good to me yeah yeah so i said i want to go with percent. Sorry, I'm thinking this is the longest survey says ever. Okay, so Charlie Brown's teacher takes for percent. I think he's going to win. There's no way he could lose with that. I don't see how he could. I covered all words in the audio spectrum. Yeah.

All right, so throw us the results, because this is apparently going to be a fail, regardless of what we do. So there's Joe's answer, which can be interpreted as, you know, it's an interpretive answer. So whatever you want it to be, that's the answer it is for you.

And then there was Alan's answer for code to music in the background at 30.0%. And the answer is take a break, go for a walk, eat lunch, anything to reboot. Now here's the rub. Technically it is the winner, right? But percentage-wise, these were both like right on top of one another. There was one vote difference between these two. Wow. Oh, nice. So they were definitely in the top two. Very cool.

And what was – was it a pretty high percentage or was it just a count of votes? Yeah, they were in like the 54% range. Very cool. And what was, was it a pretty high percentage or was it just a count of votes? Yeah, they were in like the 54% range. Very nice. And that's why when you were like, okay, aggregate that together and that would be the total. I'm like, well, that could be more than 100%. So that's not going to work. I win either way. Nice.

Everybody won this one. Yeah. Any curiosity as to what was last uh pajamas treadmill pajamas treadmill uh treadmill was last yeah surprisingly i i did not expect that one i thought that one would have done better i thought that like the idea of having netflix on in the background would be more annoying. I know people who do it. Yeah. Yeah. I can't, but yeah.

Yeah, me either. All right. All right. So for this episode's survey, Andrew Diamond from our Slack community gave us this idea, which is, after all of this that's going on with the pandemic, how likely are you to advocate for working from home in the future? Your choices are after this pandemic, every opportunity I get, or after this pandemic,

never or work from home? Is that even an option? I like those. Yep. I think it depends on how many kids you have at home. I think that's going to be the answer. Yeah, so if you have kids, then you'll say never, and if you

don't have kids, then you'll say every opportunity. Is that the thing? Right. Right. Okay. Today's episode of Coding Blocks is sponsored by Datadog, the unified monitoring platform for full visibility into all of your serverless functions. You can troubleshoot performance issues faster by seamlessly navigating between logs,

lambda metrics, and distributed request traces all within one unified platform. And Datadog provides real-time screen boards and service mapping so you can get complete observability into your serverless environment. You can start your monitoring today with a free 14-day trial and receive a free Datadog t-shirt after creating just one dashboard. Visit datadoghq.com slash codingblo blocks to learn more about how Datadog can help you optimize your serverless environment. Once again, that was datadoghq.com slash coding blocks.

All right, so on to the style section of today's podcast. So step one, have a style guide. Google has one for most of the languages they use so uh that makes sense to me if you're coding in r then good luck with that i gotta say like r has got to be the ugliest language that is actually used by people i think maybe if like if you're just like a math statistics person then it's beautiful but i don't know i don't know you've seen q sharp that's not pretty either oh is that the quantum one yeah yeah it's all the maths so uh make sure the if it's all the maths then joe should be really good at it though he might i'm an intuitionist with my math

i'm one of those people i don't know how I got the answer, but it's always right. Sometimes the greater is incorrect. Sometimes the world is wrong, but I'm always right. Just make sure. Make sure the CL follows the style guide. What the heck does CL stand for? Changelist. Seriously? Oh, changelist. Okay. uh make sure the cl follows the style guide what the heck does cl stand for change list seriously oh change list okay uh if something isn't in the style guide and as the reviewer you want to comment just put knit in there how do you guys feel about knit

we talked about that last time it feels wrong but we like the fact that there's some language that identifies that hey this isn't a blocker. Your change list can be approved. But, you know, for future reference. And we especially appreciated that Google compressed it to just three letters. Right. Yeah. I like to think of myself as like the Jerry Seinfeld

of coding. So I'm always like, well, isn't that weird that... Yeah. So I'm a big fan of knitting. Do not block PRs based on personal style preference. Yeah, definitely. Yep. Unless it violates the style guide.

That's different, right? So they basically called out there that if there is nothing in the style guide specifying what should happen in a particular situation, you can't let your personal preference block it. if there is nothing in the style guide specifying what should happen in a particular situation, you can't let your personal preference block it. If it needs to be something that needs to be put in the style guide, then you need to amend the style guide, and then that way there's a future precedence going forward, right? Yep.

Yep. Let's see. And the more you can get just into your IDE so it just does the right formatting, the better. Style changes should not be mixed in with real changes. That should be a separate CL. Yes. No, no, there's no waffling on this one. I've seen people reformat entire blocks of code, and there'll be one change mixed in with it, and you can't see it.

Nah. We've talked about this before and my preference for that type of thing where if you see that something like that is necessary and should be done, then it should be done as two separate pull requests. One,

which is the actual legit logic change that needed to happen and the other one is the formatting change and you can make them as a pull request into one that then goes into master you know yeah it should be totally totally should be should be that would be nice no yeah but i mean how many times you've been in something you're like man i can't even make my change because i can't read what the heck's going on. Let me rename some of these things, move this thing up here to the top. Okay, now I fixed a bug.

Man, come on. Are you really going to say, okay, now let me back that all out and then check this in maybe with the bug? It's so hard to separate those things sometimes. I don't know. There's only been one time that I ever denied a pull request because it was so much heavy formatting with little changes. For the most part, I'll put something in like a knit and be like, yo, please don't do this. Right. But I've only been faced with it a few times that were egregious.

I like it. You guys should see just how bad my network activity is right now. Like my, my ping times are like, usually my ping times are in like the two to four millisecond range. And every now and then they jump to like five digit numbers for a good, you know, few seconds. And every time it does it, that's where I like completely lose you guys. You need to go up there and tell your wife to get off call of duty. Yeah.

Right. That's clearly the problem. So speaking of these good, these, uh, ping times that are all staying within the realm consistency, this is, this is the realm consistency. This is, this is the next thing on the list, right? And they basically say that if, if the code conflicts, if there's nothing in the style guide or,

or if there is something in the style guide, it wins period, right? It trumps all. If it's, if what's in the style guide is, is a recommendation, not just some sort of hard requirement, then it's a judgment call. And usually you want to base it off the existing code so that you're not doing anything that is way outside the realm of what the regular code is doing.

If there's no style guide, then the code should remain consistent with the existing code. Right. Like don't start changing patterns just because you don't agree with the other 90% of code out there. And they did say they put in to dues for cleaning up existing code if it's outside the scope of the CL. So that one's kind of interesting. I don't know. I don't do too many to dues. I guess I think more of those as tickets. so I guess it is a to-do. You'd typically put in a ticket and say, come back and do it.

But again, style guide wins. If style guide's not there, then be consistent. Yeah, I dig that. Documentation. If the change list changes any significant portion of Bill's interactions test, then appropriate readme's reference docs should be updated. I love readme's. I do too.

I'd be fine with every folder having a readme. I think I would too. Honestly. It's surprising to me how little I cared about that until I started using more open source projects and you go into the folders and there's

not a readme and you're like, what is this? Right. It's amazing just how useful those things can be in every single directory. Yeah. I've never said, Oh, I could never read me. Right. Right. Exactly. That's, that's really what it boils down to. It's instead it's the inverse of, oh, man, what am I looking at here, right? So, yeah, that's beautiful. If the changelist deprecates portions of the documentation, then they should also be removed. Yeah. Sounds good to me. I should deprecate stuff more often.

I don't like this. Yeah. We've all talked about it before. We are all fans of deleting code. If that code is no longer useful, kill it. It's my favorite key on the keyboard. What if I just don't like it? Like deprecate it.

Maybe like a don't like it tag. So this next one, man, this one's the part that makes it really hard for code reviewers. And they, they labeled this section, every line. And what they mean is look over every line of non-generated source code, right? So, which by the way, for me, a lot of times generated source code, I have a hard time knowing whether that should actually belong in the repo. There are certain circumstances where it does, but whatever. But if a human wrote it, you should look at every single line of it. And they say you need to at least understand what that code is doing, which hopefully is not terrible. If you're having a hard time examining that code in a timely fashion, you may want to ask the developer to walk you through it.

And this is actually looking at the code, right? Like, Hey Joe, what did, what were you doing right here? Why is this here? If you can't understand it,

it's very likely future devs won't either. You know, like we talked about, um, last time about having, um, not, not letting the person walk you through the changes,

be like, you know, you sit there, speak when spoken to. Oh, that's kind of a cool way of like, because otherwise it's just so easy to kind of lead. So you have someone lead you through and look at things in kind of a rosier,

rosier way than you would normally. Right. I think, I think that whole thing of, of the reviewer asking the questions, it really does boil down to just that is, hey, if I don't understand something, let me ask. Instead of you just bombarding me with information because you're intimately familiar with the changes you made, let me make sure I can grok it. And that's the big difference between the two. And this is the other part they say here if you don't feel qualified to be the only reviewer

make somebody else who is qualified can come in and do the review right um that's that's fair and again they go back to the stuff that's truly um noteworthy are things like concurrency right people that that have special knowledge in that area. Security is another one. And we don't talk about that a ton on this show, but yeah, security is one of those things that you don't want to screw up, right? Because that can lead to really bad things for you and your company. Accessibility, internationalization, and some other things, right?

So yeah, it's very important that you actually review the pull request and don't just rubber stamp it. So context, Tom, sometimes you need to back up and get a bigger picture of what's changing rather than just looking at the individual lines of code. So one thing we talked about before, but haven't recently,

is having a link back to the ticket. So some sort of reference point. And I think that's probably just kind of common practice at this point. And if you've got a nicely integrated environment, you should be able to just click and get to the ticket. And I think that helps kind of give context. And especially if you can keep your notes on that issue in the ticket and have those conversations about the ticket in the ticket rather than in email or in meetings whatever then the more the better just because that stuff is tied tied together

and it's not going to be lost in some email or archived or just disappear yep also they recommend looking at the whole file versus how those few lines were changed might reveal the fact that maybe there's some other way to do that or maybe there was already a private method that did this, or maybe they're duplicating functionality or doing something in a different way. I do like the fact that they called out the fact that you, I think I just said the fact that twice. I like that they call out that you might only be seeing three lines of code, but if you were to blow out that file, you might see that that's nested inside some method that's 200 lines long, right?

Which is an obvious candidate for, hey, maybe this thing needs some refactoring, right? Yeah, but don't refactor in that same PR. Oh, no, you totally can. Absolutely can. Well, I thought you were saying we shouldn't. Oh, yeah. Okay, refactor. So I guess I didn't think about it.

Like earlier when I was talking about doing cleanup and stuff and mixing with my my changes i was thinking about really refactoring more so than no those were styles right that was a difference in styling your code versus refactoring refactoring is a different ball game which you have a good point though refactoring can muddy up what it is you're actually trying to fix right because if you if you needed to change two lines of code to fix the bug, but to make it more readable, it was, you know, 70 lines of changes. That might be an argument for two separate PRs. I mean, it depends on, like, you know, how big the issue is, right? Sure.

You know, if your refactoring effort only takes five lines, then fine. I don't care. Agreed. And a couple of questions here to ask about the code. Is the changeless improving the health of the system? If it's making it worse, that's probably a reason to decline. If it's complicating the system or if it's making it less tested, then those are probably reasons when you might want to consider. I've never seen a good PR that really shows me the percentage of tests changing.

So you can absolutely automate that one. You can say if the testing coverage percentage drops below a certain threshold, automatically fail the the pull request but how do you see like if you're just let's say you don't have that rule and you want to know if the test percentage coverage is going up or down like you kind of have to look for i don't know how to do that except for looking at the report afterwards well i mean if you don't have that if you don't have something that's you'd'd have to look at what the trend of the test coverage is over time. Yeah, I've just never seen it in PR.

Yeah, I think you'd actually have to eyeball it, right? If somebody added five new methods and there's only one – five new public methods and there's only one test, then maybe you'd have to visually do it, right? But I think what Outlaw was saying and probably what I was thinking also is you'd probably have something plugged into your build pipeline that would generate that report that you'd probably look at it in conjunction with whatever the PR is or the change list or whatever. And they also do this quote here that I really liked do not accept cls that degrade the health of the system like it's cut and dry they they don't it's not like hey if this

than that it's no you don't do it all right and uh most of the systems don't become so again that's kind of a tough one though, right? Like to know if it's going to degrade the health of the system over time. It's like if you've looked at a set of code and you're like, okay, this is a good architecture. It's readable. It's clean, whatever. You know, how do you go from that to say like, but I can tell it's going to degrade the system? I think it's an obvious thing. I think that's the key, right? If there's something that jumps out at you that says, wait, you have this thing that is in an infinite loop that's constantly

pulling or hitting something. And it's always adding things to memory. Like, you know, that at some point it's going to introduce a memory problem or it's going to introduce some sort of latency issue into your system. I think it's got to be obvious right because like you said a lot of code things are not going to jump out at you like that what about um removing unit tests that fail or rather than rather than fixing the unit tests or updating you're like ah screw these that's you can make an argument that that can degrade the health or um i was thinking too of like some sort of system you're like okay well i fixed the. You can make an argument that that can degrade the health. Or I was thinking, too, of like if you remove some sort of system. You're like, okay, well, I fixed the bug in telemetry by removing telemetry.

Like, wow. Yeah. My amazing fix broke this unit test. So to fix it, I removed the unit test. Yeah. Okay. So basically what you're saying is anytime you see a unit test that's being removed that should raise a red flag like why did you remove that unit test okay i i i like that

okay so i've got a very short list of good things to discuss wait do we hold up hold up yeah this last one's important oh well, well, I should say it. Go ahead, Mike. Okay, so since you can hear me now, most systems become complex through many small changes. Which is why they don't let these little things in that degrade. They said it doesn't matter how small it seems, it's over time that it really starts adding up. Death by a thousand cuts.

Yep, exactly. Yeah. All right, now, now, Joe, now you can jump into there. All right, we've got a very short list of good things. So if you see something good in a change list, let them know. It's okay to say, you know, positive things. A lot of times we focus on mistakes, but positive reinforcement can do just as much good as finding something bad. And this is particularly true when you're mentoring.

This is a position where you're probably giving a lot of, you know, not negative feedback, but, you know, you're constantly kind of, you know, guiding someone, which goes a long way when we're able to say something nice about it too. It makes them feel that they're actually improving and I'm sure they are. So it's, it's just easy to just focus on the negative. So I like that. So in the spirit of Google here with the compressed three letter version of saying that you're nitpicking on something, we should come up with a three letter version to say that, you know, this is actually awesome. So you know, I don't know what that would be. So just thinking off the top of my head like if you were gonna make the opposite

of knit maybe it would be like not but then that doesn't feel like it works and that doesn't feel you know like it works either wait well you did the we'll turn it up man why don't we just make it rad oh yeah rad okay i was thinking also yay or coo. Yeah, there we go. Yay. That's coo. I like it. This is where emoticons are your friends.

100. True. I do love that most of the tools like GitHub and all those do have emoticons built in now. That actually makes me very happy. Yeah. Star eye, heart eye, whatever. those do have emoticons built in now that's that actually makes me very happy yeah star eye heart eye whatever by the way i'm curious do you and i actually feel bad about this now thinking about it do you guys often put nice comments on prs like oh that's i do all the time every single time yeah i click that approve button oh i was wondering where you're going i was like dude

i never say anything nice from you on my pull request what are you talking about i approved it didn't i what about you joe uh no clicking that button i i tend sometimes i'll say stuff like you know i try to let people know but i'm sure i don't do it enough. Like, like, Hey, I saw how you did that. You know, that was nice. I appreciate that you went through the effort or that you did something and whatever. Okay.

Yeah. I don't do it much. I probably don't. And what, you know, what stinks is like, I probably am way nicer to people whose code I don't review often. And the people who like consistently do good stuff.

I'm just like, yeah, fine. This is the 12th PR in the last two days. Yeah. Yeah, it's like being married. I've been married for almost 10 years now. So it's like I'm so much nicer to strangers than I am to my wife.

Jeez. Like the barista at Starbucks. I'm like, thank you so much. Here's the tip. Even though he didn't even ask. That was so nice of you. For my four dollar coffee wow that's okay so uh we'll have plenty of links in uh this episode's show notes as well as the

mystery of how to pronounce lion fair pilgrimage gile by the way i looked at typing that i Linesmiths, Linefarer, Pilgrimage, Kyle. By the way, I looked at typing that. I initially assumed it was just like someone like splatting on the keyboard, just like pounding on it. When you look like it's gymnastics, like the letters are not next to each other at all. No. You got to really bounce around. There is an unusual amount of L's in that too. Is that like a legit name?

Dude, it is. It's a local government community in the island of Anglesey in Wales. But it's not a person's name though. It's the name of a local government community. Yeah, yeah. Like a town. Okay. It's real. It's legit. Yeah. I mean, I'm sure they just threw that in there to throw me off, and they did a good job. And you know what? In fairness, Alan, you told me to not cheat, and I just now, for the first time, pulled

up the Wikipedia link here to see. I still can't do it. And I'm looking at this pronunciation key, and I'm like, wait, what? It's got the number four in it, like four times. How do you pronounce that? You know, there's also a place in Wales called Cakeland that Jamie mentions pretty often. Well, I can pronounce that. I know.

I can definitely pronounce that. I can eat that. All right. So we're going to have some resources in this show. Yeah. Sorry. I got distracted. With that, we'll head into to have some resources in this show. Yeah. Sorry. I got distracted. Uh, with that,

we'll head into Alan's favorite portion of the show. It's the tip of the week. Yeah. Although I think it's going to change to watch Mike pronounce this. All right. So, uh, for my tip of the week,

uh, it's kind of odd. Uh, I saw a article on hacker news that had like a kind of a – it was like an article about something counterintuitive. I don't even remember what it was. And one of the people listed like, oh, you know, that's funny. I recently stumbled across this list of common misconceptions on Wikipedia. And it's just a gigantic list.

I don't know, maybe 100, 200 items that are things that are just common misconceptions, things that people think that aren't necessarily true. And so I've got a list here of a couple. And you can check the show notes on the podcast you're listening to right now in order to find this link or you just Google a list of common misconceptions. But here's just a quick list of things that's on there. Twinkies don't last forever. In fact, they only have a shelf life of about 45 days. Not in my house. Where they go bad. It's like 15 minutes on the car ride

home. See, you don't get tetanus from rust. You get tetanus from a bacteria that's frequently found where rust is also found. Have you ever heard this one where like if someone's an undercover police officer and you ask them if they're a police officer they have to tell you totally not true so if you ever heard the story about how

mary uh antoinette did not uh said let them eat cake about like a famine. That expression was actually around was never no one's ever been able to actually attribute that to her. Did you know Napoleon wasn't short? He was actually taller than average. So they called him the little emperor but they say that was more of like a term of endearment. Also he was

frequently surrounded by guards which were very much so like taller than usual because he would kind of hire big beefcakes so that's you know kind of something i thought was funny taller than average that's good and uh one of my favorites uh if you've heard the expression that people only use about 10 of their brains they say uh that is total bs anyway it's just a big list of things like that so you can just be a just crush that next party you're going to interesting facts and everyone will love you well i want to hear math whizzes percent like how much

of their brains do they do people use right well mr wizard is it percent the mobius Pi divided by negative E percent. Okay. So then for my tip of the week, this actually came up multiple times this week. And so I was like, okay, this has to be mentioned because coincidentally, this tool for Visual Studio Code called DrawIO, and I will have a link to the Draw.io integration for VS Code. Just randomly, coworkers were like,

oh man, I found this neat tool, blah, blah, blah. And then unrelated to that conversation, in the coding block Slack, there'd be other people who were like, oh man, there's this amazing new tool called Draw.io. It only was just released, at least the latest version, May 9th of this year. So maybe that's why. Like, I don't know how old the original version is. But it is really cool that you could, like, do all your drawing straight in Visual Studio Code and then be able to, like, you know, commit that in. That is truly most excellent. Yeah.

All right. So my tip, so I've been working a lot in Google Cloud of late and man, I don't know about you guys, but I've always disliked XML and I dislike it not because of what it is to me in its human readable form. Because in its human readable form, it's amazing. It's the tooling for working with XML that drives me absolutely bonkers. So I asked the question to several of my dev colleagues the other day, I think you guys included, like, hey, do you guys know of any way to modify XML in Linux? Right? On the command line. And we were all like, yeah, you said, yeah.

And said is nothing more than text parsing, right? Like it's not actually telling you if a note exists and if it exists, then do this or modify the value. And let's be clear here. Said is one step removed from reg X in terms of its readability, right? Like it is disgusting. So the short answer was no. So in Linux, there's probably a very good reason.

Most config files are just straight text lines, right? Because you can easily read and parse those things. The XML wasn't. So here's the problem. I was working with Google Dataproc. So basically spinning up clusters of servers. And the problem is when you spin up a cluster of, let's say, 10 servers, you need the same configurations to land on every single server, right? And Google has a thing called initialization

scripts that actually run on each one of the nodes as they spin up. Well, I was looking for a way to modify some of the standard configurations, like a Hive config or a Hadoop config or a Yarn config or whatever, right? And that's what I was looking for. Like, hey, can I do a shell script that'll do this? And it turns out the answer was not a simple yes. So I was like, okay, well,

I guess I'm gonna write a jar file. And then that way I can go modify these things. And that's also ugly. I was just blown away and frustrated by how hard it is to actually modify an XML file in any kind of simple form. Right. So I spent a decent portion of a day writing a little Java app, a Kotlin app to modify portions of a config file. And it really irritated me because it was like, okay, this is very much a one-off. Okay. All that said, after I spent the entire day doing it and making it work, I found as I was researching other things that Google Dataproc actually has this built into their CLI tooling.

So they have what are called cluster properties. And here's what stinks, right? If you miss one, it's not easy. If you spin up something with 20 nodes, are you going to log into every single node and then run some script to update it? It's just easier to kill the cluster and start over, which is what I did. I don't know how many times in a day. This thing called cluster properties, just know that this exists. If you scroll down on that page with the link that I've got there, you'll see this list of

various different file prefixes that they have. So typically when you're working in something like Hadoop, Hive, that kind of thing, there are all kinds of configuration files, right? They have set up this thing to where in the CLI, you can simply call out the type of config you're looking to augment, modify, add to whatever, and then just give it key value pairs. And it will automatically on every one of the nodes that spins up, go in and modify that config or those multiple config files for you on every one of the nodes. So beautiful. I didn't know it existed. That's just the beauty of like infrastructure as code though, because I've, I do find myself like it's just easier to tear it down and rebuild it.

Yeah, because if you script it out, which, by the way, I know there's lots of people out there, and we've played in Azure. We've been in AWS, Azure, Google Cloud. Here's the thing. Like Azure is really good at their UI. If you log into their console and you want to spin things up, it is super nice to do it. What you'll find is after you do that three times, you'll be absolutely sick of clicking and typing stuff in. Their UI is amazing, but you always need to be looking for that thing that says script this thing out to some sort of template, right?

Or some sort of CLI. Because then you can just hit enter. for that thing that says script this thing out to some sort of template, right? Or some sort of CLI, because then you can just hit enter. And this is where the culmination of 133 episodes of coding blocks comes together is because we have shared tips. Like the one that I don't know if you guys remember from the command line, if you hit control R on the command line, it'll allow you to search for previous commands you typed in. I use this all the time now because I'll do something like G cloud

create cluster, right? So I'll just search for, I'll do control R and then search for create. And then if it wasn't the very last one, I can do control R again. It'll show me the previous one, et cetera. Right? So that combination of things with just knowing the environment you're working with, like this thing that will allow you to edit these configuration files without writing your own applications, it all comes together. So yeah, that's my very long winded way of saying that you don't have to write your own programs to go modify XML files. If you're trying to spin up Google Data Proc clusters. Save yourself some time. I got another tip for you. You reminded me. Talking about Spark.

If you have an open source project and you want people to use it, just provide them with a Docker file. It's so much easier. I was trying to do a little bit of work with Spark. I was unfamiliar with it. I kept looking for a Spark image. There is no official one. The only way you can build the official image is by checking out

the project and then running a command it's like build docker.sh which doesn't work in windows so it's like okay well i guess i'll install wsl i'll stall java set my java home build my sh and the result of that is like a text file that a million other people have done and no one had you know built it anywhere so yeah to make it easier for people oh man that that's another thing that reminds me so i'm sorry to tail off that one but that's that's so good because what he just said about docker is one of the things that is most frustrating to me is i've seen even hadoop um or some other things like that. People are like, well, why do you want to run Hadoop and Docker? It doesn't make sense because Hadoop is essentially just a manager of other nodes.

The reason it makes sense is because as developers, you want to be able to spin things up and use the stuff, right? You're not actually setting up 12 different machines with disk arrays and all that kind of stuff to do it. You just need the environment to work so that you can test out your stuff, right? Right. So, yeah. You're trying to test out the code, not the infrastructure.

Right, right. So, yeah, there's, man, if you can make your stuff work in a Docker environment, please do because it'll actually garner adoption to your platforms. Not that Hadoop needs any help, but you know what I mean? I hear it's struggling. Right? So, yeah. Oh, man, we should pour one out.

Can we talk about this? Is it too soon? Yeah, I mean, speaking of. We don't have to pour it out yet, right? Yeah, we don't know yet what's going to happen. Right. But there was some Apache apache drill news that didn't sound too promising let's say is that a fair way to categorize it yeah i think which

if you saw the stream the live stream that uh joe and i did with apache drill um you know and and even we've talked about it on this like all three of us have like fallen in love with it. But we were, I guess, definitely late to the game. And unfortunately, it looks like the writing might be on the wall for it. But you could. So. You could, Outlaw, become a reviewer. Well, that's what I was going to say. So if you have an interest in it and you want to help that project out, you could actually, uh, you know, become an owner of that project

and a code reviewer for it. And you could have your name added to the owner's file. You could. The interesting thing was the article that Joe sent along the other day was basically because the number of people involved as reviewers right now has dropped so low that it may fall out of the active apache projects right yeah but can't it just be done that's something we've talked about a few times like what's wrong with just being done i i think the only problem with something being done is because things change so rapidly that as soon as that thing is out of favor six months down the road then nobody's going to use it right it doesn't support any new versions. Yeah, it hasn't been maintained.

So even if the thing works, right? Like even if you have an XML parser that was written five years ago, people are going to be like, is there nothing newer? Right? Like if I see something that was touched three months ago, I'm going to use that over the one that was touched five years ago. So, yeah, it does make me sad. Apache Drill is a fantastic project.

All right. So with that, we hope you enjoyed this show. Subscribe to us on iTunes, Spotify, Stitcher, or more using your favorite podcast app in case if a friend happened to give you a link or let you borrow a device to hear this episode. And as Joe mentioned earlier, if you haven't already,

we would greatly appreciate it if you took the time to leave us a review. You can find some helpful links at www.codingblocks.net slash review. Oh yeah, it's my turn. While you're up there, check out your show notes, examples, discussions, and more.

And if you are into social media, you can check out live journal. We've got a MySpace page and a Friendster. If you just search for at coding blocks, wherever you are, then you should be able to find us and you know, check out our top eight. And we've got a bunch of other social links at the top of the page.

MySpace. That's, that's where you can hear all our latest songs. Yeah.