Coding standards: Performing reviews

- During a code review, we have the author and the reviewer. It can sometimes feel like a really dangerous game of ping pong as biting comments are sent back and forth about the code, but it doesn't have to be that way. Both sides of the table can work together to make it more enjoyable. Meet Ahmed and Josh. Let's sit in on one of their code reviews. See if you can spot some of the ways they both drop the ball. - Okay, let me quickly go through this. Hm, that doesn't make sense. We never use this method anymore. What's up with using old methods? What? This doesn't match the description the acceptance criteria. Wrong, did you even read ticket? Wow. Why in the world would he use nested for loops to handle this? (exhales) Nested for loops? Really, dude? Not cool. Done. - What? Really old method. Did I read the ticket? What did-- I hate when this guy reviews my code. He thinks that he knows everything, but he doesn't. This method is not old. We use it in three other places in the app. Or did you not check? Our PM sent out the criteria in the Slack chat yesterday. I guess you didn't see it. Okay, wow. I'm not even going to respond to this one. - That didn't go well. Both Josh and Ahmed were annoyed with the exchange. Let's see how we can do better. We'll start with Josh. Instead of being antagonistic, he could've been more empathetic. If it was his code, what type of comments would he have wanted? Definitely not the ones he sent. Here are a few tips to keep in mind when leaving comments in a PR. Include at least one positive comment. Something like, cool use of extension functions. Include yourself. Why do we want to do this here? Comment in the form of a question. Have you been able to test this against staging? Focus on the code, not the person. This line confuses me. Is it meant to cue the work? Notice the difference? This is very non threatening and inviting. Now over to Ahmed. It's hard to have someone critique your work, but try not to take it personally. The focus of the review is the code and the sharing of knowledge, not you as a person. If a particular comment or question rubs you the wrong way, first assume best intent and then pose a follow up question to really draw out what the reviewer meant. And after they've taken the time to add comments to your code, it's best practice to acknowledge each comment even if it's a simple thumbs up. This lets them know that you've seen the comment and you've taken note or will address it. Let's see how both Ahmed and Josh put these tips into practice. - Okay, let me review this. Nice work with the refactor here. I didn't think this method was still in use, is there a reason we chose it here? I see a discrepancy with the implementation and acceptance criteria. Did something change? Did we consider any alternatives to the nested for loops? I've really been liking the standard libs collection functions. - Okay, cool. He appreciates the work that I put into refractory in that one class. Let me acknowledge that. I'm going to-- oh yeah the method I used for the report class. We use this method in a few other places in the app and this method doesn't trigger any of the events that we need for this ticket. Okay. Okay, great. Good catch. Our PM changed the criteria in the Slack chat yesterday. I'll ping her to update the criteria. And this one, okay. I didn't know that the standard lib had those collection functions. Okay, thanks. I'm still getting familiar with the standard lib. Perfect. - Much better. We want to have multiple pairs of eyes on our code to help find issues. Code reviews give us this power so use these techniques to make your next code review a frustration free one.