Approve My Pull Request! These Engineers Are Slowing Me Down!

Transcript is auto-generated and may contain errors.

Hey folks, I'm just headed to the office. I got some gas first so I don't have to edit this video. Woo. Um, should be an interesting week. We have some people that have traveled in from uh out of the country, so I'll be probably driving to the office every day this week, but we're going to be going to experience devs for a topic today. I don't have it pulled up because I'm literally operating a motor vehicle, but uh the the poster was talking about code reviews and pull requests and um sort of they're feeling like from their perspective uh they're kind of frustrated because it seems like people aren't really getting what they believe is the the primary reason for pull requests and code reviews. Um, and they had some interesting points, but based on like the flood of responses that came in, um, I feel like they sabotaged themselves a little bit.

So, I don't maybe this was their actual perspective and they got kind of flamed for it. But um one of the things that they end up making a statement about is kind of like, hey, I have code that needs to go into the front end and like it's really important, so like you should just approve it is kind of is kind of what it seemed like they were saying. I don't I don't actually believe that that was exactly their intention, but I think how they wrote it made it seem like look like every like if it's on fire, let me push it through um and don't actually review it kind of thing. Uh based on everyone who was responding, they're kind of like, hey, kind of seems like that's one of the things you should go figure out. So, I'm going to talk through this, give you my perspective on code reviews, poll requests, that kind of stuff.

Um, and if you're new to the channel, just a friendly reminder that this channel is driven by your questions. So, feel free to leave them below in the comments. Um, happy to try and answer if they're software engineering or career related. And of course, uh, if you don't want to leave a public comment, feel free to look for Dev Leader on social media or Nick Cantino on LinkedIn. Uh, Dev Leader is my main YouTube channel where I have more edited down videos, C tutorials. There's a podcast and live streams uh, every Monday at 700 p.m. Pacific. Come on Tesla, you can merge in. There is so much room for you. My goodness. Um, the uh, the other thing I want to call out is if you're watching this video, you've already missed the live stream cuz it's tonight that I'm recording it um, and streaming it.

But uh I'm going to be doing a live coding session where I make the landing page for code commute. That will become the new spot for uh submitting questions. So I think it'll be fun. We'll use cursor. We'll use AI. We will have it uh somewhat vibecoded if you will and I think it should be fun. Okay. So poll request code reviews. I think that um I think some of the things that they're kind of talking about in their post I I definitely agree with. Um this feeling of like people trying to get everything absolutely perfect in the poll request and this feeling of um like it it's almost like wasted time or diminishing returns. And I actually do really agree with that. Um, don't get me wrong. I think that we want to strive to be having um, you know, good high quality code in our code base.

I don't think that we want to just like put slop in. I don't think that we want it to be garbage. But I think that there's a point of diminishing returns and people uh, more often than not are, you know, stepping way way way beyond that point of diminishing returns. Um so to give you an example I think one of the things that I see like far too much of and this is over uh you know the course of like roughly what 15 years I guess not quite cuz of those years an internship I've felt pretty lucky with reviews that work well um so let's say last 13 or so years okay so still over a decade uh across startups across big tech so I work at Microsoft right now one of the most common things I see where there is a tremendous amount of wasted time is on like whites space and uh small stylistic things.

Okay. So um my perspective on this is that yes we want to have you know consistent readable code. I totally get it. Um, but I think that when we don't have these things codified, we end up having so much subjectivity brought in that you could be talking to one person and they're like, "Oh, change this so it's more readable." You talk to the next person, they're like, "Change it back so it's more readable." Um, if you're if you're talking about stylistic things or like whites space at a minimum, if you're like, "Oh, there's you have two empty lines here, like, you know, resubmit it." Um, if you're already having conversations like this, llinters, right? Get something in place to be linting.

have the same linting experience on your development machines so that you have that quick feedback so that you know right in your IDE this is our standard for readability this is our standard for styling um if anyone's wasting time on that on top of sorry anyone's wasting time on that topic on top of uh what the llinters already are doing then I would say either one you're just literally wasting time or two that's something that should probably be discussed to have put into your llinter because if that's a recurring conversation like optimize your process. It's it's it's just wasting more time. Um on two levels, one is like I don't think that it's that valuable most of the time like whites space in particular. It just come on. Um, but beyond that, there's going to be some stylistic things, like I said, that are quite subjective, and I think you're hitting diminishing returns like crazy.

However, if you do think it's that important, it's a common thing where most of the team agrees on it, but it's not quite consistent yet, codify it. Get it into your llinter, be done with it, it's just no longer a time sync in a review. I think that when people get frustrated by the amount of time that reviews take and they start talking about them or it seems like they're pushing them in the direction of like can you just sign off on this like I'm putting code up can just approve my poll request instead of can you review it and get the feedback. Um I think that when this happens it's uh it's basically a frustration of what I mean my perception of this is it's a frustration of like the value that's being added. Okay. So if people are putting code up and they're basically like you know the mindset is like it sucks to put it up for review.

People are just going to waste time on this. I'm just going to get like nitpick kind of stuff and like just man just approve it. I'm putting it up. It's good. It works. approve it. I think if you're in this mindset already, you're not getting the value out of your poll requests that you're probably striving for. Okay, so one, like I was saying already on like the stylistic stuff, like honestly, I think that's the lowest value that you could possibly contribute to a poll request. Um, because a lot of that can be automated through llinters. So that's a low value thing that takes up a lot of time. I'm not saying that there's zero value. I understand consistency, readability, but the the return I feel like is ridiculously low for the time that goes into it. I think that if you're if that's where review comments are like starting and stopping, like either everyone's got like amazing code and everyone's on the same page, which is incredible, super cool.

Um, but I I think that that's pretty rare to have in teams. Okay. So, I have been fortunate enough to work with the same small group of people for, you know, a good few years. I don't know, five five or so years, like straight. And our our reviews basically got to the point where we're reading them and the feedback is like it's so minimal. It would be like, "Hey, nitpick. Um, you have a typo here. Hey, I see you added these test cases. Um, what about this one or something?" But like they're very small. Not like hey like you got to go redesign this thing. And I think that probably the more common case is that you have this type of situation where aside from the stylistic stuff people are like hey this isn't the right pattern or we've moved on from this or uh this is risky because it doesn't consider X Y or Z.

Um, if you're finding that you are in this state and you're getting frustrated by pull requests and code reviews, like in my opinion, that is one of the huge value ads. But I think there's like a good like buzz phrase for this. Um, I think it's shift left is I think what people call it. um instead of getting to the point where you're putting code up for review, it's in a pull request and then for the first time people are going through this and being like, "Hey, I know you just spent a week on this and it works from everything that you've done uh and try it out and you've written tests for it, but like uh don't like this pattern or this pattern has a flaw or or whatever." Right? Basically, go back to the drawing board for some or all of it. Sorry, got someone driving like a complete idiot in front of me.

If there's a lot of space in front of you and you're riding your brake, maybe you shouldn't be in the fast lane. Uh, so if you're finding that you're in that kind of situation, I would recommend that it's not um it's not that reviewing code is is the wrong thing or it's bad that you're getting feedback. I think those are all really awesome things, but I think the frustration is that again, it's like it's like wasted effort, okay? It's like the amount of amount of time and effort that went into something and then the return that you're getting. It's like the wrong mechanism. I think they work really really well when everyone's on the same page or you're trying to get people up to speed to get in the rhythm of stuff. But the the challenge is that if you go off and write some code, you're designing a solution for something and then the first time that people see what you're trying to go build is after you've spent like days on it.

I don't know what the even the right threshold is. Like I could imagine being frustrated after hours, but let's let's go with days, right? You've spent you start on a Monday and it goes up for review on a Friday. And now you have your colleagues being like, "Oh, like I don't know, man. you should probably rewrite it this other way. That's going to feel like You wasted a whole week. And I shouldn't say completely wasted. There's probably a lot of value, but it's going to feel not great. Go back to the drawing board. We need to consider a different design for this. You implement maybe the concept is right, but the implementation is sort of not what people would recommend based on design patterns and stuff that are established. So, what do we do about that? Well, back to the, you know, the buzz phrase like shift left.

Like, how can you have that conversation sooner, right? You can use the same tools and have the conversation sooner by doing like a draft PR, right? Do a little bit of it. Have like a couple of classes or functions or something you push up and explain like this is the direction I'm thinking about going. um that might not even be a good tool because you want to have a discussion about it and maybe the amount of discussion that has to go into those choices like writing them back and forth in a comment on a review even though it's draft is not effective. So maybe you do a like a lightweight design dock or just like a you know brainstorming session and talk through it. Maybe you do a hybrid, right? Maybe you get your draft pull request up. It's like super lightweight. It's like a skeleton of the approach you want to take.

You don't have like a lot of the methods implemented. It's pretty empty or you have pseudo code written down like in comments or something. I don't know, like super lightweight. And then you go have a meeting about it. So, you send it out in your draft and you say, "Here's how I'm thinking about doing this." And like, you know, you're already anticipating there's going to be some some push back or some concern. Here's the link. review it. You know, we'll have this conversation tomorrow after people have had some some time to review it. Uh and then we'll walk through it and like raise any concerns. Maybe you send it out and people review it asynchronously and there's no feedback and you can cancel the meeting. But like point is how can you get this feedback sooner without putting a lot of effort in, right? What's the minimum amount of time and effort that you can put into something to get the feedback you need that it's in the right direction?

And I would just urge people to consider this kind of thing regardless of, you know, I'm giving you some examples of how you could like physically go implement it, but I don't I don't mean to prescribe that. I just mean to put the ideas in front of you, right? It's that's a a couple of ideas for how you could go approach it. My point is, if you're finding that there's a lot of waste of time and effort in a poll request, try to get the answer sooner. Right? It's the the way that you uh might be trying to lean into your tools like poll requests and code reviews, the way that you're trying to leverage them might just not be effective. And it doesn't mean that they're bad. It just means that they're not being effective for how you're trying to use them.

So you can either improve the code review and pull request, you know, process that you have or look at adding in other things uh that that kind of supplement it so that you by the time you get to the pull request and code review that those problems are mitigated or reduced. So I think that's my major piece of advice here like to I don't know to like to summarize it right is that these types of processes I think can be extremely beneficial for getting consistency in the code base for making sure that there's the right eyes looking at things um you know spot checking stuff all sorts of added value. Now you might say not the right tool for those things. That's fine. I'm just saying I think it is a tool. I think it can add a lot of value. But I also have seen like time and time again like super ineffective usage of code reviews.

And like I said earlier, the number one that I see that's a time sync is just like stylistic stuff. Um, and I wonder, you know, maybe this person who wrote this Reddit post, maybe that's kind of they've seen more stuff like that where the the value ad from the reviewers is so low that they're like, why do we do this? That's why they're saying things like, I got to this is important. I got to push it through. Like I like I I get that feeling. I just think that it's a it's kind of poor poor wording. uh and uh the reasoning is not really uh justified in that case because it's not properly explaining what's actually happening. Um what else? Um, I know some of the comments that were coming back on like like from other Redditors were around this idea of like uh pu, you know, push it through.

We'll fix it later. And everyone's like, oh yeah, like you know, good joke. There's never a fix it later. Like this is like the end. Once it goes in, it'll never change. make a ticket to file for tech debt and like it's always going to be scheduled like the last thing. Um, and I get it and I want to just give you like a maybe a different perspective. If you were to land that code and it wasn't the best, right? Like it's it's good, but there's like this idea of like, hey, we can follow up and um and make it a little better for next time, right? and then people are up in arms about that there isn't going to be a next time. Um maybe a different perspective on that if that work is always going to be prioritized the lowest is maybe it's because it's actually not important.

I know that's like people aren't going to like that, but maybe it really doesn't actually matter. Right? I just want to put it out there. Then you could obviously extrapolate what I'm saying and and say, "Well, if this keeps happening, we're going to have a million different patterns. The code base degrades, man, your codebase is going to degrade anyway." That might accelerate it. I'm not I'm not arguing that, but my point is like if you're very upset that like your tech and this is a whole different conversation. If you're upset that your tech debt doesn't get addressed, there's pro I've made videos on this. There's probably a good handful of reasons for it, but one of them is perhaps that from a business perspective and the time that goes into that, it might just not be worth it, man. And I think that sucks for a lot of people to hear.

I I say this like with confidence too because I've been an engineering manager for just under 13 years. But the first eight years that I was an engineering manager, I was also writing code with my team, right? I was also in the code base actively working with, you know, the other engineers on my team. I was writing code across teams. I was in some of those conversations with other teams where you know having conversations with the product owner, some of the project managers and um you know having conversations with the developers and the PMs where they're like developers want tech debt, PMs aren't scheduling it. What do we do? And like the biggest thing there is like the PMs don't understand the value of doing the tech debt, right? and the engineers can't explain the business value of doing it. You're not speaking the same language.

How the hell are you ever going to expect a PM to prioritize tech debt that you've not explained the value of over a new feature or or bug fix that they know they're getting flak for? They're they will never do it and I don't blame them. So if you're like, "Okay, we're gonna back to the code review example, like sure, we'll go address it later." And then you're all sour that like that tech that's never going to get addressed. It tell us what the value is. So my take on that kind of stuff is like I agree if you have patterns that are completely divergent or you've you kind of moved away from one. So, let's say it's one thing to introduce a new pattern because I think that if there's merit to it, that might be something that you you say like this could be the new pattern going forward.

But if there's an old pattern that's being reintroduced, like someone copied it from a spot that is legacy cuz the patterns have diverged, then I would say that might be good grounds for um for like, hey, like we should go back and and try to clean that up. But again, it's like what's the what's the ROI on that? Because if it's quick to go back and change it, sure. Um, if it seems like more of a nitpick, then sometimes I would say like I would literally say in the comments, if you're already going back to correct some other things, then go address this, too. Um, I generally like my code review comments are mostly like if you're not going backwards on, you know, design decisions like stuff that we've kind of deemed like that's not a good pattern, uh, or it's not completely busted like or super risky like you've just missed like you've missed the mark on it.

Most of my comments are going to be nitpick and it's like I don't honestly I don't give a if you go back and fix them. Be like for next time I have to sneeze. Pardon me. Double whammy. For next time like it might be good to keep that in mind. Like I would encourage you learn from this one next time go do it. Um and only if you're going back into the code to go fix something up then consider it. But, you know, I I make it very clear in my comments like this is just a nitpick or and and then like don't block your review on this or basically like I think that this is really risky. But I find that those types of comments these days are uh you know a lot a lot less frequent. So, be curious. I'm almost at work. You know, I think everyone has different perspectives on code reviews and poll requests.

Um, at the end of the day, we're trying to build good software. We're trying to make sure that we can keep our agility up in terms of developing. Um, we want to make sure that we can keep our code base from falling apart sooner rather than later. Uh, it's always going to happen though eventually. Degrades and degrades over time, but sometimes you can keep it together. So, be curious in the comments if you have different thoughts on this. Um, would love to hear. Always looking for healthy debate. So, like if you're going to be rude about like I think you're stupid and whatever and like here's your your own perspective, like that's probably going to get blasted cuz I don't like being insulted. But if you uh are open to a constructive conversation, I love hearing different perspectives and sometimes I'm reading when people have comments and I'm like, "Oh, I agree with that.

Maybe how I communicated something when I'm just kind of stream of consciousness talking here. Maybe I didn't do a good job or I I forgot to mention that or something." So, u it's always really cool to hear different perspectives. So, and uh I definitely appreciate all of you that are active in the comments. It's really cool to see people that come back, watch the videos and and have uh you know their own experiences to share, right? I always try to remind people like I am just one dude, you know, my goal is to help provide different perspectives, but I only like I got mine and I do my best to try and share others, but it's not always going to be captured. Like this video was probably pretty biased, right? But that's why I do invite you to share. But I think that's it. We should have a bunch of videos this week.

And I'll see you next time. Take care.