LaurieHWhile many developers dread doing code reviews, I realized several years ago I actually enjoy reviewing code when I approach it like a sport. Since no one's code is perfect, I challenge myself to hunt down issues to keep our code efficient, bug-free and easy to follow.
Ok - you're probably thinking, "Geez, I would hate to have my code scrutinized by her if her goal is to find something wrong". But, I've had many developers seek my reviews after their initial introduction to my pull request feedback.
No Rubber Stamp Approvals
As a seasoned (read: OLDER) developer, I am well aware that my code is not perfect. I count on the code review process to find where a null check is missing, or where logic could be more efficient. Or, heaven forbid, I have a glaring bug. I am passionate about code quality, so I will never rubber stamp my approval on anyone's PR - even when I'm reviewing code for more experienced developers.
"The first thing I want to see when I find a bug in our application is my approval on the PR" - No One Ever
If you're looking for flaws while reviewing code changes, you'll either find something, or learn something. Either outcome is valuable.
Follow the Logic
I always try to follow what the code is doing when I'm evaluating someone's work. Skimming the code doesn't do anyone any favors - seek to understand what's happening.
What is this variable and where is it set?
Does the name of this method match the functionality?
Is there a more efficient way to get the same result?
Could this duplicate code move into a separate method?
That's a cool approach - I never thought about doing it that way (see... I learned something new)
Ask for Clarification
When you're in the zone during development, you can easily follow what your code is doing. But, if you hand that code to someone else they may end up very confused. Or, you may come across your method months later and wonder, "Who wrote this garbage?".
If you find yourself confused (even for a minute) while evaluating a code change, you should absolutely ask the submitter to make adjustments. Sometimes simply renaming a variable or adding a comment in the code will help with readability (and everyone will appreciate the clarification when they're troubleshooting later).
Be Kind
I've yet to meet a developer who has written flawless code, so there's no sense in berating your coworker when you find an issue during a review. Uncovering problems is what you set out to do in the first place! While some people have been put off because I rejected their changes due to a typo or misspelled word, I assure them my feedback is strictly about quality.
When I review someone's code for the first time, I always explain that I'm thorough and ask them not to take anything personally. I also request they scrutinize my changes since I'm expecting them to call out my mistakes. Peer reviews are a valuable part of the development process and I want everyone on our team to have high expectations for the quality of our code.
Scrutinize, Scrutinize, Scrutinize
The definition of scrutinize is to examine closely and minutely. When we scrutinize code changes during the pull request process, we are ensuring code quality - and that is the goal.
