Vokal Code Review Protocol
What Is A Code Review?
Code review provides each engineer the opportunity to learn and double-check their work. A code review should not be a chore, but a privilege. Use code reviews to give honest feedback, learn new things, and reinforce quality.
Remember each engineer is 100% responsible for the quality of their own code. The same goes for reviewers—each reviewer is 100% responsible for the quality of their code reviews.
A code review should take no more than 90 minutes. Give honest feedback, ask questions, and be polite. In general, the reviewer should consider the following:
- Check for tests! Any pull request without tests must be rejected
- Enforce code style (per platform: iOS, Android, Systems, Web)
- Check error handling
- Consider memory management and possible leaks
- DRY
- Concurrency
- Security
- Performance
- Portability
The reviewer should approve the changes when they are satisfied with the pull request. The addition of a joyful emoji or "LGTM" ("Looks good to me") doesn't hurt. If the reviewer has questions about the review, use the regular comment feature. If changes are required, choose "Request changes" in the Review Changes dropdown.
After at least one approval (without any outstanding requests for change), a Senior Engineer will merge the pull request. Do not rely on Senior Engineers to review all code; there are generally plenty of engineers on your team to aid you in code review. Also, there's nothing wrong with getting more than one reviewer for your pull request—more eyes increase the chance that errors will be spotted before going into the next build.
Note that Product Owners will not merge changes; only engineers are empowered to merge changes.
When Is A Code Review?
A code review is mandatory for each and every pull request. An engineer should open a pull request for each feature. It is the submitting engineer's responsibility to ask a colleague to review the pull request. You should know how your fellow engineers prefer to be contacted about a pull request: @mention in GitHub comments, instant message, email, etc. Understand that your colleagues may not be able to respond to a code review request right away.
Where Is A Code Review?
Code reviews are generally done entirely on GitHub, but can be in person on a shared screen. The depth and quality of code reviews are up to the submitter and the reviewer. If the submitter wants to cover a class or method, or verify that a test covers all relevant functionality, then it is their responsibility to work with a reviewer.
Why Should I Do A Code Review?
Code reviews are time that gets billed to the client (use the "<clientName>:Code Review" instance in SpringAhead), and billing time is how we get paid. As an engineer at Vokal it's your responsibility to take the time to do effective code reviews. If you're an Apprentice or Associate Engineer, you still should do code reviews—in fact, "Review code effectively" is an item on the Apprentice Skill Checklist.
As was said above, use code reviews to give honest feedback, learn new things, and reinforce quality. Code reviews aren't just about finding mistakes that need to be corrected. Code reviews are an opportunity to see what your colleagues are doing and what techniques they're using, to share knowledge.
Code reviews are also a perfect time to ask questions about patterns you haven't seen before or that don't make sense to you. Even if the code is "correct" (by some definition), if it doesn't make sense to you, it may need clarifying comments or restructuring. Even if the code doesn't need to be changed, it's worthwhile to have another developer explain it so that the knowledge is shared.
Code reviews are a bi-directional channel for information sharing.
Pull Request Courtesy
Please be kind to your reviewer. Pull requests should be as granular as possible, ideally including just one feature. The goal is to make the review as easy as possible for your reviewer. Only add additional commits that address comments during your code review; do not add commits for new features while a pull request is in review.
Tips for a Harmonious Code Review
It behooves reviewers to be humane as they comment because plain text is a blunt instrument. This pull request has many examples of "bad" interaction, starting with the first comment. Here are some tips for a happy code review:
- Couch your opinion with phrases like "I think", "I feel", or "In my experience"
- Cite your sources whenever possible
- Use personal experience
- If there are many people involved in a debate over a single line those involved should speak in person
- Remember you are critiquing the CODE not the CODER
- In general try to ask questions rather than making statements
- "What was your thinking here?" is much preferred to "This is wrong"
- Even "Why do you feel that way?" is better than a simple, accusatory "Why?"
- Don’t get picky, you don’t have to find an issue in every review.
- It's ok to be positive and say "Great work. Good to pull!", or "Hey buddy, LGTM.
👍 "