Code reviews
At Storio Group code reviews are practiced to share knowledge and increase the quality of our work. Every piece of code that reaches your main branch or production should be peer reviewed, whether that is through a formal pull request or a pair programming session. In this document we share some guidelines on how to make code reviews productive but more importantly an engaging process.
Preparing your team for code reviews
Make sure everything can be reviewed
Identify whether there is enough expertise within the team to have everything reviewed by at least one other person, but ideally multiple. This can also help you identify knowledge gaps.
Make sure reviews are done frequently
Agree with the team when and how often code reviews are carried out: Once or twice per day encourages project velocity and prevents people from having to ask for reviews repeatedly.
Make it easy to see what you need to review
Everyone should be able to easily see what they should be reviewing: Use Github review requests, CODEOWNERS files, Slack bots or some other tools to get an easy overview of the work to be reviewed. GitHub even supports automatic code review assignments with some setup.
Review your code reviews
Discuss the code reviews during your sprint retro: Is everyone happy with how the team is doing them? Does everyone feel they contribute to code quality and velocity?
Preparing your projects/repositories for code reviews
Eliminate noise during code reviews
Using CI/CD helps you prevent questions that would always be asked such as “Have you run the tests” or “Is the build succeeding”. Using a linter and/or formatter will prevent any discussions around code style such as indentation levels or variable names, and also helps developers to be more productive by catching small mistakes early. Testing also plays an important role in making code reviews effective. By using a combination of unit, integration and end-to-end tests you can always be confident that changes have less unexpected impact.
Set approval rules on your project
Approval rules provide clear direction on who needs to sign off on a pull request before it can be merged. This can prevent accidental merges and also shows clearly who is best suited to review a pull request, which can be useful if you are receiving PRs from outside your team.
For simple changes, you can consider simpler process like Ship/Show/Ask [1].
Organize your code for collaboration
Collaborating on code works best when git can easily merge your changes. If your project frequently deals with merge conflicts it may be a sign that the code should be refactored to ensure changes to different parts of the codebase do not affect the same files.
Automate pull requests notifications
You can configure Github and/or Slack to send automated messages to your team when a PR is open or merged. This way the team gets notified whenever there’s a code to be reviewed.
Submitting your code for review
Review your own code first
It is tempting to submit a PR whenever you feel done, but you owe it to your team to review your own code first.
Keep it small
Code changes are easiest to review when the change set is small. To achieve small change sets for large features you could
- Divide your total changes into separate commits that are easy to review by themselves
- Use feature branches to have small changes reviewed that are not ready for release/the main branch.
- Use feature flags and API backwards compatibility to merge changes that are not intended for release yet
- Separate functional changes from non-functional changes (such as refactors)
Explain your changes
Provide explanation about the why and the how of your changes to the code. Ideally you can relate the pull request to a JIRA ticket and/or some metrics/logs. If something turned out to be more complex than initially estimated, mention this. Giving a good explanation is also essential to providing context to people who later need to investigate your code, which is a good reason by itself to provide some extra information.
Be clear about what you want from reviewers
Not every pull request has to be merged to the main branch: You can also use code reviews to get feedback early, but make sure to mention that in the description. You can also use the draft
flag for this.
When reviewing code
Be kind and concise
Your code reviews should be factual and easy to process. Feedback should not be personal (Compare “This code is ugly” vs. “This code does not conform to our style guide”).
Be precise
Github allows you to place comments on specific lines or blocks of code, which makes it easy to see on which line feedback is given. If your feedback refers to something outside of the pull request, make sure to include a link or other reference.
Be pro-active
Sometimes small errors slip through with obvious solutions. In such cases you can use Github’s suggestion feature which makes it easy to apply your fix. In general if you have thought of a solution while reviewing, you should share it.
Check your own feedback
Even when something may seem obviously wrong, it is good practice to check whether there is actually an error. For example if you think a deprecated method is being called, you can first verify this. This will also automatically give you a source that you can add to your feedback.
Allow for disagreement
At times it will not be possible to get something exactly the way you want it. Be prepared to accept a compromise or agree with your team on suitable follow-up steps, such as new tickets or a follow-up pull request.
When receiving feedback
Take feedback gracefully
Receiving negative feedback is never fun, but it is important not to take it personal. Give your team members the benefit of the doubt and understand that code reviews improve the output of the entire team. If you disagree with feedback you have received, be patient and reply in a constructive manner.
Ask questions where needed
Actively ask for input when you are not sure on how to proceed with the feedback you have been given. You could request a pair-programming session or ask the reviewer to take over certain parts of the code.
Process feedback timely
Just as reviews should be done regularly, it is also important to process the feedback at a similar pace. Let your reviewers know if you are working on pieces of feedback and whether they are done.
Inspiration
- How to Make Your Code Reviewer Fall in Love with You: https://mtlynch.io/code-review-love/
References
[1] Ship/Show/Ask: A modern branching strategy: https://martinfowler.com/articles/ship-show-ask.html