Dealing With Harsh Code Review Comments (Why Passive- Aggressive???)
If you work for some company writing code, you've probably dealt with code reviewers. These code reviewers aim to find flaws in your code, usually with the objective of improving and making the overall system works better.
However, this is not always what happens.
Sometimes, these code reviewers are complete idiots and they tell you nothing that matters. They don't make efforts to provide decent feedback and, end up being complete bastards. So, what should you do in these situations?
I recently got a question from a subscriber and he asked me the following question:
“I recently started with programming for Front-end development and working on my first project started 2 months back. A Code review was done last week and the feedback which i have got have 18 comments. For half of them I consider myself responsible as those comments are about formatting things and some redundant code, but latter half of them are just because of exiting logic which somebody else worked for same project and I have applied same to new functionality I am adding to the application.
And comments are slightly rude like strict commanding statements, expressing aggression about how long it took for him(code reviewer) to understand the code.
Could you help me how should I respond to this as this is first time for me and I really don't have any idea that would I be able to fix them all making sure code doesn't break down.”
How should you handle passive-aggressive or even aggressive code review comments? Watch this video and find out.
Working With A Legacy Code Book:
Transcript Of The Video
John Sonmez: What's up. John Sonmez here from simpleprogrammer.com, and today I'm going to be answering a question that I got here, that I have on my Trello board. This is about code reviews actually, so if you want to know about code reviews, this is a hot topic for some of you. Stay tuned, 'cause that's what I'm going to be talking about in this video.
What's up guys? If you're just joining me for the first time, I'm John from simpleprogrammer.com. Simple Programmer is all about the soft skills. It's about how to make more money, how to deal with your boss, your coworkers, how to get along and advance in your career, and market yourself, and all that kind of good stuff that you need to know to really be successful as a software developer. So click that subscribe button. Make sure that you mash it really, really hard and click the bell to get notifications when the new videos drop on this channel, 'cause you're not going to want to miss 'em.
So he says, “Hi John, I recently started programming for front-end development, and working on my first project. Started two months back.” Okay? “A code review was done last week, and the feedback which I have got have 18 comments. For half of them, I consider myself responsible as those comments are about formatting things and some redundant code, but latter half of them are just because of …” I think it's “existing logic, which somebody else worked for same project, and I have applied same to new functionality. I am adding to the application. And comments are slightly rude.” All code review comments are slightly rude, or they're at least passive aggressive. Like strict commanding statements expressing aggression about how long it took for him to code review, which I understand the code. “Could you help me? How should I respond to this, as this is the first time for me, and I really don't have any idea how to be able to fix them all, making sure code doesn't break?”
Okay. So a few things here. All right. Number one. Attention to detail. Exiting logic? Yeah, I think you mean existing logic, right? Some grammatical errors. Guys, capitalize your damn I's. Seriously, this is just … if you type this, okay. Here's the other thing, right? And this is relevant, guys. I know that you're like, “Oh, you're picking on him. He's not an English as a first language person.” I get that. I get that. But still, it doesn't matter. You're in a professional setting. It doesn't matter, right? These are things that you should be aware of. So if you type this into an email program, into Chrome in an email program or any email program, it's going to have spell check and there's going to be a squiggly line under this. Right? I mean, if I do this, won't I get a squiggly line? Well, maybe not in this view. But come on, you can turn on spell check. You can turn on formatting. Especially if you're going to send this to someone professionally.
So I'm not saying this to pick on you per se, but I'm giving you some feedback. And I'm not being in a passive aggressive way, I'm just telling you straight up. This is really important, okay? And you're going to get dinged in code reviews because of a lack of attention to detail 'cause it really is important. I mean, I understand what you're saying, okay? Because I am a human. I can interpret language even if you use the wrong word. A computer doesn't. Okay? If you write code that is incorrect, they're not going to interpret that, and also it's more confusing. Like, if someone's reading your code, they should be able to understand that code. It should be very clearly formatted. All those things are really, really important. Part of my answer to how you should respond to this is going to be based on that.
Formatting, redundant code, all this stuff are things that you can automatically handle in the IDE. You should never get dinged for stuff that can be automatically checked, right? It's like the same thing I say, “Should your unit test ever fail?” No, they should never fail. Why? I mean, when you're checking code and committed to the bill, right? Why? Because you can run the unit tests on your machine before you commit your code, before you check it in, and see. And if they're failing, then you haven't done your due diligence, right? The same thing with QA test cases. Like if you have test cases, the QA's going to run regression tests against your code. Why didn't you run it initially? Anything that you could have prevented, you shouldn't have a problem with, right? And a lot of people have this issue, right? It's like I always tell people that work for me, “Double check things. Triple check things. Test it.” People will make mistakes and errors, and you're going to make mistakes. Okay?
But the mistakes you make should be the kind of mistakes that you make because of something that you did wrong, not something that you didn't see or that you looked over. Does that make sense? Right? So the mistakes should be ones of actual error of understanding, of logic, of doing the wrong thing, as opposed to mistakes that are attention to detail mistakes, things that could have been prevented. You want to prevent all the preventable mistakes as possible. Okay? So that's one thing. And you say you got 18 comments on there. Half of them you're responsible, so you took responsibility, so that's good, about formatting, things [inaudible 00:04:59] code. Okay. A lot of them are because of existing logic, which somebody else worked for the same project applied some new functionality. I'm adding those. So just because the existing logic in the application is bad, it doesn't give you any kind of excuse, right? Maybe you've made some mistakes. You have to be real careful when working with legacy code. There's actually a good book by Michael Feathers called Working with Legacy Code, I believe. You should read that book for sure.
Let's talk about now slightly rude, strict, commanding views. Expressing aggression about how long it took for the code reviewer to understand the code. This is the kind of crap that you're just going to have to deal with, all right? I know you're a new software developer. You're new to this, but there's a lot of egos in software development. A lot of arrogance, a lot of people that just trying to prove they're better than each other. I can go into the psychology behind this, but I'm not going to go into depth. But to be honest with you, right, let's just be bluntly honest. A lot of people that get into the software development field … and you should know this, right? Even though it's not really politically correct to talk about, are guys that were picked on in high school and were nerds, and they have something to prove, right? I see a lot of ego and arrogance. I'm not trying to judge. I'm not trying to say that they're bad people.
I'm just saying that if you … this is the kind of outcome that you get from someone who's been treated this way. And unfortunately, we see a lot of that in software development. Because if you don't have as many friends, if you're kind of an outcast in society, you go and you play video games, and you're on the computer, and you learn stuff, and you generally work on your intelligence and improve in that area. Not a judgment. I was like that. I was totally picked on as a kid. Are you kidding me? Okay, now I've got like five subsets. They don't fuck with me. But before then, I was totally picked on. Okay? And this is why I went into software development, because I was a little shy, nerdy guy. Don't take this personally, just understand that's just what you're going to encounter in this space.
Not to say … I mean, there's some great software developers that are well-adjusted people who went to Tony Robbins seminars, but there's a lot who have not and need to just go to unleash the power within, and just hug people and jump up and down, and stuff like that. So, okay, you should take ownership for everything. Okay? Especially taking ownership for the things that you could have prevented, the formatting. Don't apologize. Never apologize. What should you do if you don't apologize? You should own shit that you do. You should take ownership for stuff. So you should say, “Hey, I messed up.” But not only saying that you messed up, but correcting it, right? This is why apologies are worthless. Apology says, “I'm sorry that I got caught and I have to pay the penalty for it.” It doesn't mean anything, right? So apologies would actually be effective, even though I don't condone them.
I just condone the actions of if you were to make an effective apology, you would say what you did wrong, what you're going to do to fix it in the future, and how you're going to prevent it from happening again, and what you're going to do to fix it right now. Okay? So it's the same thing with a code review, okay? We're talking code apologetics here. Okay, you don't have to say, “I'm sorry I'm a little bitch,” or something like that. You don't have to do that. But you can say, “Look. Hey. You're right. I take full responsibility for all of this stuff. Okay, here's what I did wrong. Bam, bam, bam, bam.” Don't even say why you did anything wrong, because that's an excuse. Instead say, “Here's how I'll prevent this from happening again. I'll use the IDE auto-formatting. I'll slap myself in the face if I ever have a lower case I when I'm proofreading my text, so that I'll feel the pain.” Okay? Whatever it is.
You'll say, “I'll do this, I'll do that, I'll do this.” Okay? And then you say, “Here's what I'm going to do right now to fix it. I'll do this, I'll do that, I'll do this.” Okay? Now I went through this stuff, okay, with my courses. I did a lot of courses for Pluralsight. I did 55 courses for them. And there were times where, oh man … if I'm going to be honest with you, I made courses, and there was some jealousy. So some of the authors that reviewed the courses were authors that didn't like the fact that I was doing so well on Pluralsight and making so many courses. Honestly. Okay, and they're like, “Who is this guy?” Whatever. I was overthrowing existing hierarchies, okay? And so they came down on me. Oh my. The kind of comments, the nitpicky, nitpicky stuff that I got. But here's the thing. You have to recognize when someone is in a position of power over you. Okay? And you have to act appropriately. And so, I was pissed. I was like, “Oh, what the … how dare you.” You know, argumentative.
But you know what I did? I just said, “Hm. Okay. Yep. All right. You're right. Mm. Mm-hmm (affirmative). Mm-hmm (affirmative). Mm-hmm (affirmative).” And I fixed all the stuff that they suggested. Even stuff that didn't need to be fixed. I mean, some of it was valid concerns. A lot of it wasn't. Some of it were things where I was right, and I decided, “Well, this is not the hill I'm going to die on.” There were a few hills to die on, but when I got to those, they were so … they gave me a hundred negative comments and things to fix, and I say, “98 of them, yes, I agree. It's my fault. Okay, I'll fix it. Good point. Thank you for pointing this out. Yes sir. Can I have another?”
But then two of them I say, “Look, here's why I did it this way. I'm not going to change this one even though your suggestion is an adequate one. But here's what I'll do. I'll do it like this, and I'll make this little bit of a change, but I'm pretty much keeping the concept. I did this on purpose.” Right, that kind of thing. So that's exactly the same thing I'm going to advocate with you, is I'm not saying to just completely capitulate when you think that things are wrong. But if there's things, fix everything that you can, agree for the most part, right? Pick which hills you're going to die on. If there's a couple of hills that you're going to die on, then die on them. But make a good logical argument, not an emotional one. Just be like, “Hey, this is why I did this. This is what I think about this. Here's my understanding and reasoning and logic. Does this make sense to you? Or …”
I mean, you got to ask the question. You got to say … you do it in a way where you're asking them for their input, right? Instead of being combated with argument. What you want to do is when you have a differing opinion with someone, is you want to present your interpretation of it, and then throw it over to them and say, “Look, here's what I was thinking about this. And I did this, 'cause of this and this and this, but you know, I could be wrong. But what do you think?” Right? And now if you've made out a logical argument and you haven't put the ego into it, and you haven't said that they're wrong, and you've said, “Well, this is why I did this. Does that make sense to you, or what do you think? Am I doing the right thing here?” Right? Then they're probably … now they can do that without losing face.
They can say, “Hey, oh yeah, yeah, yeah, yeah. Actually, you're right. Oh, oh, oh, oh.” Right? And they might even say … they might even become magnanimous and be the bigger man, and say, “You know what? I was kind of wrong when I said this about you, and I actually said that kind of in a harsh way.” Bam. Now you've got them eating out of your hands. The last thing I'll say is this, is that this may not work. You may just do all the right things, and people are just assholes. And if so, just develop [inaudible 00:11:46] mindset. Just be like, “Mm, okay.” Just deal with it. Right? And then find another job if you have to, but just realize that's just how life works, okay?
All right guys. That's it. Go check out Simple Programmer. Go subscribe. Go watch more videos. You know, whatever. Take care.