These Developers GATEKEEP Code Reviews!

Transcript is auto-generated and may contain errors.

Hey folks, I'm just driving to the office Monday morning. Oh, I just I just realized I didn't submit my video to post it on code commute. Damn it. Um I have two videos ready to go up today. Oh, that's frustrating. Anyway, whatever. Even more. Um there's going to be a ton of videos this week cuz I'm driving to the office every day. Uh because we got some guests coming in. going to experience devs for this topic. And this is about code reviews and basically people being um what feels like people being too pedantic or being like gatekeepers on on code reviews to the point where it's not helpful. And so, uh, the the poster on experienced dives, I'm not going to get into like the specific details of like their their exact situation cuz I want to talk about this more generally cuz I've seen this.

I lived through this. Um, this has certainly shaped how I approach code reviews, especially as someone now that I'm managing engineering teams without actively contributing to the codebase. I I need to make sure that I take a stance that um like that my comments come across very clearly when I'm leaving them. So for for folks that don't have a ton of experience um you know in in different uh team settings, different companies, that kind of stuff. Um this might be kind of weird to to kind of talk through and navigate if you don't have like some examples to be like, oh yeah, like I have kind of seen that or yeah, that does kind of suck. So um in a in a perfect world, you know, we have code that goes up for review. And like I said, in a perfect world, it's uh small commits.

Things are understandable. Um you basically have team members that are reading through it. They can understand it and to the point where everything's so clear to them that like they don't have comments about how things work. All the style stuff is followed. All of the conventions for like different architectural patterns are followed. You have all the right tests. People can see how it comes together. And like basically the the review almost feels like a waste of time because uh everything is so clear. I mean it kind of sounds ridiculous, but in a perfect world that's kind of how it would go to the point where you'd get to this situation where you're like, why are we even reviewing code if it's like if everything always comes together so perfectly like this? But the reality is like that's not that's not real life. That's not how things go.

Um for many different reasons. And I don't This woman's This woman's got a sign out the front. It says, "Caution, children at play." And she's just like waving at people. Uh there are in fact no children at play. Um but that's okay. So maybe she is a child at heart. And um because we don't live in a perfect world, we have to think about like well what's the goal of the code review or the poll request review and like how do we how do we make this effective? It's yet another tool that we have in our tool belt as software developers, right? So there's lots of different benefits to to code reviews and pull requests. Um you know ideally ideally we don't have to rely on humans to catch things but that is like one of the benefits of code reviews. Now another benefit is that you can educate people about what changes are coming in.

So you can keep the team informed on the technical changes so they can see how the code base is evolving. It could be a learning opportunity for people on the team that are putting up the code like hey I'm trying my best to follow the patterns but like this is a new pattern we had to introduce or I'm new to the team and I'd like some feedback on if I'm doing the right things. So there's all sorts of benefits, but like I said, in a perfect world, it's almost like the code review starts to feel like extra work because you have all of the communication up front. People know what's coming down the pipe. Everyone knows the patterns and best practices. It's just like that's just not it's just not real. It's not like you might get to that point on a team.

I've certainly had a team where we were very close to that, but there were still things where like periodically we would catch um you know escapes or something like that and it's like it means that there's some value being added even when you have you know maybe uh all of those other things kind of reduced to low value. Now, if we know what the goal of the pull request or a code review is, now we can try to think about like how do we make these go more optimally? And unfortunately, like this Redditor was writing about, there are sometimes people on teams that are so hyper vigilant about kind of like nitpicking everything that goes through that what happens is that there's like a it forms a huge bottleneck. Okay? And it's not that what they're saying is no value.

It's not that and like it's impossible to know and make it this a perfectly generic statement across all of these situations but you know there might be situations where they are doing a really good job calling out things that like hey you that needs to be changed right like that's risky or that's not the pattern we follow or you know you'll get people that are they're so hypervigilant about chasing down like whit space or like you know a variable is is named well but It's like it could be even better and to the point where you're like, "Look, man, I can't even I can't even do my job. Like, I can't do anything because any line of code I write, I have to go rewrite it." Like, that's exactly how it feels. And then you multiply that across everyone else on the team. And it's like you actually end up having effectively one person gatekeeping everything that's being done on a team.

Now, yes, there are things there are situations where you might have a lot more newer team members or a lot more junior team members. And there's a lot of benefit to being able to catch a bunch of stuff like that, but statistically that's not the norm. It's not the norm that like every single line of code that someone writes needs to be rewritten. And like yes, we can always improve code, but if every single line of code you're writing like the comment is like this could be better, this could be better. It's like yeah, everything could always be better. So the goal like you have and you need to establish this with your team like get on the same page for what the goals are for your pull requests or else you'll be spinning your wheels there and have a lot of friction probably for not a lot of great reasons.

There's a class of things that I feel like you can make a lot of progress on in terms of like reducing friction for for code reviews. That's basically to be like styling and and linting code, right? Like I cannot tell you historically how many times I've seen people basically like rejecting reviews or blocking them for like something like whites space like there's trailing whites space on this line like not even not even uh your indentation's like really crazy and like I can't read this code but like trailing whites space the end of the file end of a line um you Oh, someone left a comment and there were two spaces somewhere in the comment and they're like, "Well, no, that's like we got to go back and change it there." If you put linting in place where you can have your IDE do this, you can do it with like commit hooks.

There's all these all sorts of different things that you can do to not have to worry about this and have consistent styling. This can dramatically improve the amount of time that you get back from what I would call like extreme bike shedding in programming. There's so little value in spending time on stuff like that in my opinion. I'm not saying that like you know well Nick here's an example of a whit space change that actually helps. Like I'm not saying that there are zero. I tried to give you an example of even like, you know, if you have crazy indentation that's not consistent and it's like I it's actually misleading to see, you know, if things are like logically grouped together, I get it. There's always going to be situations, but when you have all of these other scenarios and it's like, look, like if everything else is good to go on this and you want to gate me, like I, you know, we have things like automatic merges and stuff like that.

Imagine a pull request where everyone signed off on it before the end of the day and then you have one person who basically denied it because of whites space. Now you miss the integration build. You miss overnight where it gets deployed out like this depends on the types of products and services you're doing. Now it's missing like multiple deployment trains to be able to start running in production because because two extra spaces or like white space at the end of a line. Like it's insane. And like that doesn't actually make a difference. Like it's not like suddenly that's not readable. And if you're the kind of person who's like, "Oh, well, when I'm looking at diffs and like the white space at the end of the lines, like dude, adjust your tools. Like don't make this everyone else's problem." So, invest into linting. Have some uh code styling and standards.

Yes, there are still things that can come up with this. So, I I've seen people where uh the rules become too strict and now you're constantly kind of battling against them and it's just because like it's not consistent with the team and what the team actually wants. So, I'm not saying it's a perfect solution, but I think that it's the the lesser of two evils. And if you can get a team that converges on a a set of things, at some point, at least in my experience, you end up having like the team not really debating on this stuff anymore until you start seeing new patterns emerging and it's like, wait a second, maybe we need to adjust this or you're introducing uh some code that is truly an exception. I don't know.

I've had this with like um integrating with third party libraries or or something where we're like hey look like we have a seam in the code and we have to do some funky stuff around that seam and like that's breaking our linting but it's constrained only to this spot and like we need an exception to this. But that being an exception versus the common place means that you're not spending a ton of time getting blocked on on what I would just see as dumb Okay. So, linting out of the way. The other thing that I think is really important is like, and I wanted to talk about this because someone in the comments of this Reddit thread was like they had said something about like, you know, I use uh conventional commit messages. Um, I think that's what they called it. There's like a spec for it.

I am actually not even aware of this, but I can I can imagine sort of what it would look like. and they're saying that way I can, you know, people know if I'm leaving comments that like actually matter for the for the code. And someone commented and they said, well, like what do you mean? Like why would you leave a comment if it doesn't matter? And I think it's kind of just like slight miscommunication there. But I think what they're trying to say is like if I'm leaving a comment that's like I need you to change this or like we should talk about this because there's something that doesn't really sit well with me, that's the kind of stuff that I think you want to draw attention to and and and talk through. But making that obvious, right? Making that obvious versus like to give you an example when I'm going through stuff now because I kind of said like I'm not really contributing to the codebase, right?

But I still will do reviews for my my engineers when they when they ask for it when I'm going through that kind of stuff. Like if it's C in particular, like I write a lot of C code and I have for for a long time. So there's things I can pick up on specifically to what I would call, you know, writing C well uh at least not terribly. So there are some things where I'm like, "Hey, like here's an opportunity." And I will explicitly say like this is a nitpick and like, you know, or things where I'm like, "Hey, like this this could be improved, but like if you're not going back to like make other changes, don't let this be the one that blocks your review." So I tried to make sure that I'm educating. And that's one of the things that I wanted to touch on based on the comments in the Reddit thread.

Like why would you say something if it's not actually, you know, going to be gating and it's that reason. It's education to say like, hey, look, like here is an opportunity. It's not going to make or break your current change. Like I don't want to hold you back. And I know you're going to be working in this area going forward. So like the next time you're coming through here like something to think about is like you know naming things this particular way will make it more clear or um we don't you know in this file we haven't done a good job with like some of the naming conventions a little weird and you're following that so that's fine but next time you're coming through like maybe we should think about that a little bit more. Um just like things like that that aren't gating the current work.

I don't want to gate the curve of work, but I'd like to inform people and give them options, right? So, I I really like that education part and being able to write those things down and that way people can say like, "I actually disagree with that." Right? They can say like, "No, that's this way for a reason." Or they can say, "Oh, I didn't know that." There's other people on the review that that are reading it. They might go, "I didn't know that." Right? So there's just like I find a benefit to having more communication that way, having it documented. Uh again, especially if it's like little things. It's not like leaving a comment to be like, "Oh, it's not a big deal, but" and then writing like a novel where people it's going to are going to want to debate it or something like that.

These are just things where it's like you could tidy this up. You could clean this up. Consistency things. we can make this better in the file or or this area of the code, but uh I think it's a really good opportunity for education. I want to cuz I'm most of the way to work now, uh halfway or something. I want to talk a little bit about like what do you do when someone's like basically being an and it's not clear or they're gating the entire team cuz this is this is like I've lived through this. It's real. It's painful. Um the first thing to recognize is that someone is trying to help. It may not feel like that. in some moments they might feel like they're pissed off and like they're getting frustrated cuz they're like why does this keep happening and like I keep having to leave these comments like people aren't getting it.

So like it can it can be really challenging not only for the people on the team but even for the individual that's that's kind of doing it. um they might be oblivious to it and um that's a thing too, but I I think ultimately I just want to remind people because it often doesn't feel this way. People are not trying to be malicious. It's extremely rare that you have someone that's just being an because like they get excited about being a piece of to people. There's probably some other motivation and I just want you to keep this in mind as you're going through this kind of stuff because if your immediate reaction is to also get defensive and to get frustrated now you have potentially two sides that are feeling that way and you're going to be headtohead or you're going to be getting frustrated.

you're going to want to go on the offensive cuz you're fed up and this person doesn't actually even realize that they're that they're, you know, becoming a bottleneck or a pain in the ass for others. So, I understand that these types of things can be frustrating, but please start this like this journey by trying to remind yourself people are not being malicious. They are just trying to help. I think when you go into things like this and you frame things that way, it sets you up to try understanding why people are doing certain things. Okay? So, instead of just trying to go, "Nope, I got to tell Johnny that he's an and like I'm fed up with him blocking my poll requests or you know, Johnny is the worst on poll requests. I'm just going to stop adding him.

That way, I can just avoid him altogether and I can finally get my work done." Instead of doing this, if you try to understand like Johnny keeps doing this on on poll requests, like it's always he's nitpicking whites space or like these little things that I don't get. Like again, like why is he doing that? Well, you could go talk to him. Maybe it's a matter of like, hey, look, like we don't have uh linting, Johnny. Like if you maybe this is something that you could push for, right? Like Johnny, if you keep, you know, I see that you're spending a lot of time looking at like these whites space changes and stuff like that and like seems like right now a lot of people don't seem to really be bothered by that. So it's creating overhead, but like you seem to really care about this, so like I'd like to understand more about that.

Oh, it's your your diffing tools or oh um whatever the reason happens to be great. Like thanks for explaining that to me. Cool. like this ends up costing us a lot of overhead in terms of like our context switching or whatever else and like I wonder if there's a better way like could we do you know could we run a linting tool or could we do something else to make sure that this stuff is automatically done and that way you don't even have to go looking for this stuff anymore it just kind of happens like you know what I mean like I'm just giving you one example with like the white space and the linting but if you seek to understand why people are being frustrated. You can literally partner with them on trying to solve problems with them.

And that's easier said than done because the hardest part is like trying not to just get frustrated with the other person and then trying to make some space to actually talk with them cuz we are terrible at this. It's a it's a difficult thing to do. So, I don't mean to minimize it and say it's trivial, but like honestly, we could solve or improve a lot of situations by actually just talking with people, but it's a lot easier for us to like avoid or to like talk about it without them because we're just, you know, it's avoiding a difficult conversation. So, that's one example. Um, another thing is like, uh, and this is maybe pretty obvious, but if you're noticing that people are, uh, you know, writing like novels, in poll requests, and like there's a lot of debates back and forth, get on a call.

If you're in person, talk in person. Uh, get it hashed out. I have found that many many many times people are roughly on the same page. You know, there's going to be situations where someone didn't know about something, of course, but I've seen people go back and forth debating things, arguing, and it's like they talk about it and they're like, "Oh, like you were actually saying something pretty close to me and I was saying something pretty close to you and like now we can see that and now we can focus on like, you know, dive into the detail about what we didn't agree on, come up with the path forward." But it's uh sometimes people get into these these code review comments and it's really just about how do I prove myself right and the other person wrong and it's it's not productive. I'm sure many of you have experienced this.

Like catch it early and just be like, "Hey, cool." Like message them on the side. Be like, "Hey, can we let's get on a quick call, talk about it." Just like think about trying to get to resolution. It doesn't matter who's right or wrong because the point is your code is blocked. And if it's not your code, it's the other person like their code's blocked. Like you have your team is blocked from delivering progress because you guys can't agree. So the goal is just to get unblocked and start moving forward, right? It's not the goal is not who can be the most right. It just it just doesn't matter. What else? Um, I've had to, you know, for what it's worth, I've had to have a conversation with an individual in my career. Um, and this was like a learning experience for me as a manager.

And I want to share this with you because I think that number one, it's like it's a really good learning for me, but I'm I'm hoping that you can take the lesson and and apply it with and you don't have to be a manager for this at all. Uh it's it's a feedback kind of thing. So, I've worked with someone in the past that um that they were kind of like this on on code reviews and stuff and truly their intention was not malicious. They were like, you know, they from their experience they've had to go back and fix a bunch of things when people didn't understand it. And so they would gatekeep a lot or they would like rewrite people's code and just be like, "Oh no, like I did it, now it's better." And like kind of unsolicited, but there was a lot

of stuff that was like you're like, "Man, you got to stop doing that." And like people would not want to put him on to pull uh pull requests and code reviews. Like it just became a really big problem cuz he would be the entire bottleneck for the team. And then what would happen is people would either suffer through it or they'd be like, "Well, screw that." Like, "I see he's the bottleneck. How do I make my way around this?" And then sometimes he would find out about changes and go and rewrite them. So, it was like there's a problem here. And I think as a manager, I I let that go on for too long. Um, you know, I was I was observing it because I was also contributing to the codebase. So like I felt that personally and then it's like well is that a me problem?

And even if it was just a me problem I should have gone and talked to him about it and I didn't originally. So you know lesson like part one of the lesson like that was me not doing probably what I should have and then I was getting some feedback from individuals and it's like okay like it's not just me. Okay. And then so now I know it's not just me. I know that um there's other people on the team experiencing it and um I think like I started to learn like you know I'm getting frustrated of course cuz I'm living through it but from like working with him I'm realizing that he's a lot of the time doing this because it's like he doesn't trust the people on the team not because he he thinks they're evil or sneaky or something. He doesn't trust their experience and like he has a lot of pride in the codebase.

So that means when something's wrong, he's like, "Well, I need to be responsible to fix it." Like it's coming from a good place. It doesn't seem that way on the surface when you're looking at these things. You're like, "Dude, this is rewriting my code like without telling me." and like, well, I'm gonna keep having to dodge him and not add on to code reviews because I don't want him doing that to my stuff. You just create this like, you know, this growing void, this separation of people on the team instead of coming together and he's doing it cuz he doesn't have a lot of trust. He wants to make sure that like, you know, he's going to be the one that has to go, you know, solve the problems later anyway, so he might as well make sure that it's it's done right. air quotes uh done his way so he understands it like that kind of thing.

And I let it go on for too long. And uh by the time I gave this individual feedback, you know, I can I can remember this conversation even though it was many years ago, not the exact specifics of it, but I can remember like one of probably one of the hardest conversations I've ever had. And I've had to unfortunately let go of people, too. And this was this was really hard. And I think that it was so hard because I realized like it's gone on for a long time. and I haven't brought it up and I had to give him kind of this you know this feedback where it's like hey like we got to talk about this because you know and I had said something like there's a lot of feedback coming from the team where people are getting frustrated and uh they're finding it really difficult to work with you on this kind of stuff.

They feel like they can't collaborate with you because like basically it's it has to be done your way and there's no other way. like you end up being the bottleneck for this. And I can remember it wasn't like he wasn't like immediately like angry or like like no no no like you don't get it. He was kind of just sitting there like and I was like oh my god like I could tell he was just like completely destroyed by that feedback. like he was he was hurt because like I said his intentions were always to try and help and I just remember like it he was kind of sitting there processing and he was like I I just wish that someone would have told me sooner. Wasn't like up in arms trying to like argue with me about it. was just like I could tell he was torn apart and he was like I just wish someone would have told me sooner because he felt embarrassed by it like he he wanted to be helping.

He thought he was helping and here the feedback is me doing a shitty job of giving it to him like hey basically everyone thinks that you're hard to work with and you suck for it. Like that's not that's not good. And what would have worked a lot better is giving him that feedback along the way, trying to catch it early, trying to be like, "Hey, let's let's not make this a thing." So, I encourage all of you that are listening or watching, like, if you're going through stuff like this on your team, please try to have conversations with the person sooner. You don't have to be a manager at all. You don't have to like sit there and take it and be upset and then like only let your manager like like let them know and wait for them to do something about it.

I can promise you I can promise you that if you have a conversation with a peer about this kind of thing, it will be so much more meaningful that you do it. And when I coach people on my teams, I will give them similar types of setups. I will say, look, like if you're have for any sort of interpersonal challenge, like I'm happy to support you on this. I can make sure I can dig into this a little deeper, understand from the other person's side, I can if you need me to do it, I can talk to them about it in some way that you feel comfortable about. Maybe that's an like keeping you anonymous, whatever. But I I always say like I can do these things, but I highly recommend that you have a conversation with them. And if you feel uncomfortable about that, I would personally much rather spend time like coaching you through that so you can have that conversation than than the initial part I proposed.

But I always kind of give them that option and say, "Look, like if it's at a point where you feel like you can't do anything and like this working relationship's very bad, I'm like, I can I can step in to go help." I just think it's significantly less effective. It might it might seem like effective on the surface where it's like, "Look, manager comes in, manager's going to diffuse the situation, blah blah blah." Uh, and then you might be listening to this and being like, "Well, Nick, otherwise, it just sounds like you're trying to avoid doing your job." No. Um, what I'm trying to do is coach people to have those conversations so they don't need me to to mediate future conversations. I'm trying to coach them so they have the tools to do this so that they can do it early, they can do it often because this is the kind of thing that will always be there in your career.

You will always have these situations where there's some type of friction. You know, code reviews, projects you're working on, anything. There's always going to be this kind of stuff. So, I would much rather give people tools so that they can navigate that. You don't need me for that. I'm happy to help, but I would rather give you the tools and help you practice that stuff, guide you through it, give you confidence, so that you don't need me or anyone else, right? This spot is terrible. Why do I park here? One sec. But anyway, some things to think through about poll requests, code reviews. I hope that's helpful. Um, I realize it's kind of all over the place, but something to think about. So, thanks very much for watching. If you have questions, leave them below in the comments or you can send them in anonymously at codemute.com.

There's a contact form there. Check the box. It says anonymous. Happy to try and answer anything career related or software engineering related that I can. I'm just a guy. I got experiences and opinions and I will try to give you as much context as I can from my experience and uh hopefully find it helpful. So, thanks again and I'll see you in the next one.