What are code review guidelines? What’s a code review?
A code review is a process that enables you to present a part of functionality or feature to other people (usually, team members working on the same project). In this article, we’ll discuss our code review guidelines in order to help you make the most out of them!
The main purpose is to allow coworkers to investigate what authors have implemented and assist them to produce a better solution, more maintainable code and to verify if the code fulfills both the business and the development requirements.
Thanks to it, everyone gains something:
- Managers — they can often view screenshots or video of how a given feature looks, this helps to understand in which phase the given feature is
- Author (submitter) — opportunity to learn from other team members
- Reviewers — knowledge about other parts of the project that they may not even touch anywhere; an occasion to learn from an author and other reviewers
In addition, developers can improve their knowledge of the best practices, error detection and identify vulnerabilities.
Before you submit a Pull Request (PR)
Ensure your PR is small enough to be reviewed honestly. Actually, this point requires you to plan in advance. If you expect the feature to be big, either it should be split into smaller parts (in terms of requirements/user stories) or it should be made as a base PR with base logic and other PRs which will extend this functionality.
Here’s a perfect tweet which says everything about it:
2. Prepare your PR as you would like to see it, you may want to consider the following checklist:
- How did it look before? (screenshots, videos)
- How does it look after changes? (screenshots, video)
- Read the diff on your own — you may find some places to fix immediately!
- Link to relevant requirements and ensure all of those are fulfilled
- Explain the shared code changes and flows if necessary. It may happen that your feature required the code that already exists in the application be changed. It is always a great idea to call the guy who implemented it (by the way, talking about your approach with other team members can help you avoid heavy changes after a better proposition is presented)
- If necessary, prepare a draft PR which will show any heavy changes which could have an impact the whole application
- Ensure that clever, sophisticated or strange implementation has some comments in the code directly so that others know why you selected this approach over other available options, it will let others know what you tried and why it wasn’t sufficient
3. It is always great to have CI involved, automatic linter checkers and tests runners should always be running on the PR to ensure that no tests are broken and style-guide your team had chosen is kept.
4. Ensure you have at least written unit tests and tested your solution on a few devices and browsers which you are supporting in the project.
5. If necessary, go through the checklist your team requires and has agreed on, it may be understood as a ‘common issues’ to avoid repeating the same issues over and over (even though it may never have happened to you).
6. Communication is the most important part. Never take anything personally:
- Nobody wants to attack you
- Always try to search for a consensus
- Never blindly follow the reviews in the implementation
- … If you don’t agree, ask for an explanation and present why you think your idea is better with relevant sources
- Show respect and expect it, there should always be a constructive discussion instead of “you cannot do X, do Y”
- Don’t be toxic, if a reviewer ask for something, even if it’s relatively easy and self-explanatory for you, explain it, your team will only benefit from this
- It relates to everyone involved, make your own code review guidelines or use existing examples as a starting point. Example: https://github.com/thoughtbot/guides/tree/master/code-review
Reviewing the Code
- You’ll surely know the image below and you should already know why commenting in the PR using similar terms is a waste of time (yours, other reviewers, just everyone). Avoid submitting comments that have 0 constructive points.
- Focus on the most important parts, always try to prioritize the importance of a given issue you have found.
- Verify requirements, ensure that no point was omitted and everything works as your Acceptance Criteria and Testing Scenarios say.
- Don’t argue over details. Don’t escalate issues that are not worth the effort. Sure, everyone wants to have a perfect codebase, your comment and explanation should be enough. In case of war, ask your team leader to assist.
“Quibble over details. Since you didn’t prepare, the easiest things to bring up in the review will be the ones that jump out at you on the screen. Is the code formatted using a different convention than what you use? Snipe about it. Are there some misspelled words in the comments? Obviously, that’s worth mentioning. Go ahead-tie up six software engineers to haggle over grammar.” ( TechBeacon, 2018)
- Check as if you were an author of the code. Your team is responsible for the product as a whole, therefore by approving the code you are signing that the code fulfills your team code review guidelines and meets the standard your team approved. In other words, you are a co-author
- Think of a product as a whole while reviewing, not a single part, don’t focus only on given feature you are reviewing; rather consider this is a part of your product and it ensures that no convention is broken (both UI and code flow). Think how dynamic changes in other parts of the application can imply on this one or how ‘extendable’ it is.
- Code coverage — agree with your team if you require any particular coverage
- Tests — as above; focus if they are valid at all, not just if they are there.
- Commenting a code — explain why, point to documentation. The same points as mentioned for an author under communication.
- Don’t cross the border, if you have a lot of PR(s) or code to review, or one which is too complicated, take a break; adjust the time to your needs, usually working on it for longer than 30 to 45 minutes will reduce your focus and thus the feedback quality
After review: author duties
- Respond to every single comment (with comments if you need to clarify/discuss) or just use a ‘thumbs up emoji’ if you agree
- If something was fixed, provide a link to a particular commit
- It is not a war, you are a team and you are working together — to ship better product and learn from each other
- Make a consensus with other team members
Time pressure (“Ship it!”)
Remember, these code review guidelines are a tool (!) that help your team keep:
- The quality of the code
- The quality of the solution
- The possibility to easily extend the functionality
- Learning from other team members
Bearing that in mind, include a Code Review in your estimations and planning. Assume that 10–15% of additional workload and adjust this value over time you have more accurate measures.
You will always be under pressure to deliver faster and your management may not understand that “even simple features sometimes take a long time”.
- It is always hard to explain that code reviews are necessary unless you link to this article!
- It is always hard to prove that without a proper code review your client will find more bugs.
- It is even harder to explain that you need to include refactoring in your planning (as it’s likely that a code review won’t be accurate enough on its own).
There is no universal solution for how to explain code reviews to client in numbers or by using business language but you should never omit them, as it’s worth it.
Magic keywords to help you:
- QA process — it cost less to find & fix the issues before the feature is merged
- Risk reduction — as this is a part of the QA process, it reduces the possibility to hit the issues when a feature lands on the staging/production
- Investment in the codebase — less technical debt is introduced
Notes from the author (and developer):
Definition of Done
Please keep in mind that the guidelines above are just another vision of the Code Review process.
Define the ‘Done’ statement with your team. You may pick some of the points from the above and add yours according to the way you and your team like to work. Once agreed, never let it go.
You may set your own set of rules and definition. I kindly ask you to only keep one of them in heart:
Code Reviews are there to help you create great products you’re proud of and to improve your own skills. By following our code review guidelines you’ll be an expert in no time. Use them wisely, in a constructive, non-toxic way.
We’d Love To Hear From You
Is there anything you’d like to add, have we missed anything? If you’re interested in hiring an epic development team, drop us an email (hello@appnroll.com) so that we can schedule a call.
Please feel free to share your experiences with us in the comments below or via social media (send us some photos or videos too), you can find us on , , , and , let’s connect!
To learn more about App’n’roll , take a look at out our and our other posts . If you enjoyed reading this article, please share and recommend it!
All images appearing on this blog post are the property of their respective owners.
Originally published at https://www.appnroll.com on June 6, 2018.
