Skip to main content

Performing Effective Code Reviews

No matter your role or skill level on the project, performing code reviews as a team member or individual contributor to any project is very important.  Performing code reviews is a valuable tool for all parties involved. It allows the reviewer to provide their thoughts or feedback on a topic and it allows the author the author to receive feedback on documentation or code that they have written.  The question I find a lot of people asking, including myself, is how do I know that I am providing an effective code review for the author of this contribution? The answer to this is not always a clear cut path, but there are some recipes that I try and follow when giving code reviews. And that is why I wanted to write this post, to provide insights as to how to approach an effective code review, and to provide examples of situations I have encountered along the way from the context of private team based and open source code reviews.

 

Team Based Code Reviews

Team Based Code Reviews

In the context of a reviewer as an individual contributor or leader on a private team based project, code reviews can take on a very crucial aspect of the development process.  Code reviews act as a mechanism to teach, communicate, and share different opinions on how code or documentation should be written in a project. Private team based projects often take a different approach to reviewing because of the smaller audience involved.  Here are a few steps I have found to be effective when performing private team based code reviews:

1) Reviewing the overall commit as a whole. Does the commit achieve what it was set out to do and is this goal clearly communicated in the commit message?  For example, a lot team based commits will be tracked against something like Jira, Pivotal Tracker, or possibly GitHub. Looking at the commit message and the code from a high level, does this code solve what is spelled out on the ticket?  If not, an effective way to communicate possible missing pieces is to ask if there is more to come on the commit or if the commit is a possibly (WIP) work-in-progress?  By asking about the commit it allows the author to not take misinterpret your tone, gets the question answered, and provides a mechanism for this initial feedback loop to occur that other team members can see as well.

2) Reviewing the actual code in the commit.  This is often the most difficult and lengthy part of the review process.  In this part of the review I evaluate the code for any issues that I can easily identify as they may not have been apparent to the author.  For example, items I am looking for might be an issue I uncovered while I was developing a similar ticket, or just syntax or testing edge cases know to me to cause issues.  If there are identified issues, one of the most effective feedback approaches I have seen is to provide new or historical code samples for the author that relate to the identified issue.  From there, ask the author’s thoughts on the code or how it might interact with the system if the commit were written differently due to an experience you have seen or identified. Most all developers can resonate with this feedback and it shows that you are willing to get invested in this commit instead of providing a one sentence fix-it message without any more context.

3) Running tests and checking for coding conventions.  One of the last items in a private team code review that I like to do is running unit tests, code linting, or checking for coding conventions.  In this step identifying possible regressions or issues before the code gets merged is an important last step in validation. Any issues identified here can also be identified by a smoke test or build system pre-merge stage.  At this point the code is ready to be approved or identified as needing further attention by the author. However, every once in awhile, if a PR is crucial and requires all hands on deck I will set up a development environment and test the feature out myself just as a way to provide a hands on smoke test before the code is approved.  I should note that hand testing the commit is usually the exception and should only be done when a commit is crucial.

 

Open Source Software Code Reviews

Open Source Software Code Reviews

In the context of performing a code review on an open source project, code reviews can take on a much different experience from that of a private team based review.  Here are a few valuable tips that I have learned from performing code reviews for large open source projects:

1) Contributing Guidelines and CLA's. Most large open source projects have contributing guidelines and some have CLA’s that you need to sign before contributing.  Please make sure that you take time and completely review these guideline and understand the contributing process before making a commit or reviewing a commit.

2) Project Goals. Large open source projects often have members and maintainers that manage the direction and development of a project. If you are providing a review as a contributor in one of these projects then it should be a review that is in line with the maintainers goals for the project as well as your technical experience and feedback. Otherwise you can run the risk of providing feedback that will need to be addressed by one of the maintainers as well before it can provide value to the project.

3) Reviewing the actual code in the commit. If you are providing technical feedback on piece of code, like private team based projects, I like to provide a code example on why my thought process is valid.  This can be a test case or a code snippet that explains your point of view. Like team reviewing, this also let’s the author know you are serious about your feedback.

4) Provide documentation as the basis for your suggestion.  In highly visible projects there is often a set of standards or regulating bodies that manages either protocols or a working group around a project.  If you have feedback, try and backup your feedback with references from these regulating bodies. Providing a reference means you have researched the problem and have found a documented solution.  This will be noticed by the maintainers and the other reviewing bodies as well.

5) Open source contributors are volunteers. Always remember to approach all contributors and authors with the understanding that these contributors are 90% working on these projects in their spare time and are not paid for their efforts.  Always make sure to provide positive and constructive feedback while still getting your point across.

In Summary

In summary providing valuable code reviews is a great skill to have no matter which role you have on a private team or in open source project.  Performing an effective review can mean the difference between a project making progress on a feature or teaching a team member something new. Code reviews are also an effective communication style on your technical opinions, so always remember to approach these subjects with courtesy while still getting your point across.  Thank you very much for reading and I hoped you enjoyed this article. If you have any question comments or concerns please make sure to leave a comment and I will try and get back to you as soon as possible.

Member for

3 years 9 months
Matt Eaton

Long time mobile team lead with a love for network engineering, security, IoT, oss, writing, wireless, and mobile.  Avid runner and determined health nut living in the greater Chicagoland area.