How Do I Give Great Code Review Feedback?

You got the job, went to work, and now you’re being asked to review someone else’s code!  For the first time, code that someone else wrote depends on your thumbs-up to be pushed into production.  It’s nerve-wracking, to be sure.  You want to make sure the code is good–but it was written by someone with more experience than you, so how could you ever possibly critique it?! How do you give great code review feedback?

…or…

You got the job, went to work, and now you’re opening your very first pull request.  For the first time, code that you wrote will be pushed into production–but first, it’s got to get the thumbs-up from other developers on your team, including your boss.  It’s nerve-wracking, to be sure.  You want to make sure your code is good before putting it out there–but you’re not exactly sure what “good” looks like. How do you put your best food forward?  

There are a few things to keep in mind when reviewing someone’s code, or even when reviewing your own code.  The things on this checklist are common things to look for when reviewing code, and answering these questions may be helpful to you when you’re asked to leave feedback on pull requests.

Comments should be helpful, specific, and actionable

It is important to remember that you are not your code.  If someone leaves you a nasty comment on a pull request, it’s not a comment about you as a person or about your programming ability; the commenter may only be having a bad day or they might be a poor communicator.  Take a deep breath, fight the urge to be nasty right back, and ask for clarification on the issue with your code, as politely as you can manage.  Similarly, you should do your best not to leave nasty comments on others’ pull requests.  Always try to find at least one positive to say about the code you’re reviewing, even if you have plenty of critical feedback to provide. 

Comments should always be about the code, not the developer, and any feedback given should be helpful, specific, and actionable.  “Fix this” is not a helpful comment; instead, point out exactly what the issue is and try to suggest another way to approach the problem.  If your comment is nitpicky, you can still offer advice, but others will appreciate it if you acknowledge when you’re simply having a difference of opinion–and don’t push for a change to the code based solely on your personal preferences for how something should be written.

What to do if something is hard to understand

If something in the code review is hard to understand, say so!  If you can’t understand a piece of code at all, the code review is your opportunity to ask how something works (or how it is supposed to work).  Remember, the purpose of a pull request is to catch any issues before it is integrated into the rest of the codebase.  As a new developer, your fresh pair of eyes and willingness to ask questions is a valuable asset to the team when it comes to reviewing code!

Questions to help you come up with great code review feedback:

Readability 

• Are there a lot of comments in the code that could be removed, or are there unnecessary comments left in the code? 
• Are names descriptive and accurate?  (This includes variable and property names, class names, method/function names, and file names.)
• Is a consistent coding style used throughout the code?  Does it match the accepted conventions used in your workplace? 

Security

• Are connections properly closed/disposed of? 
• Is input properly validated and/or sanitized before being used? 
• Are passwords or other sensitive data being properly encrypted or hashed? 
• Does the code provide any noticeable avenues for security attacks? 

Architecture & Design 

• If the code is object-oriented, are OOP principles followed?  Is code properly encapsulated and functionality abstracted? 
• Does the code utilize modular components or services?  Are methods written in such a way that they only do one thing?  Is code repeated between components? 
• Are any design patterns or other programming conventions implemented accurately? 

Automated Tests 

• Are unit tests included in the code review? 
• Are the tests accurate to the expected functionality of the code? 
• Are there other test cases that should be included in automated testing for this functionality? 

I hope these questions help you come up with great feedback on your next code review!  After a while, answering these questions when you review code will become second nature, and you’ll be giving feedback on pull requests like a champ.