We always want to be fast at code reviews...
How frequent is it for you to be reviewing code at 3 am? When code reviewing, do you find yourself thinking: “I mentioned this before.. We should have some sort of process”.
We keep learning a lot with our user base which currently supports more than 200 000 developers.
A big aspect is how developers waste themselves in the process of code reviewing. Code reviews (pull requests, commit validation or approval), can be tedious and exhausting. We’ve gathered some aspects from people who are doing it right. These are small hints that we’ve seen from our users that highly help so you don’t have to be alone at 5 am reviewing your team’s code.
1: Review less
Smaller commits lead to smaller more manageable code reviews. Besides other clear benefits (1, 2, 3, 4), dividing work into smaller chunks leads to more understanding of the intent of the change (hence more understanding into ways to put it better).
Problem: My team likes to have big bulky commits.
Solution: Start dividing your work into smaller commits. When your team sees the clear benefits of reviewing your code, they will most likely follow.
2: Time boxing
Code reviewing tends to be secondary in the process. Hence, it’s common to see it slipping away through the day when finally you’re all alone in the office validating work from other people.
A good way to help mitigate this is by assigning a time slot in the day to review the code. When that is not possible, try to apply a maximum value to it (like 60,70,80 minutes).
Problem: My team ships code into production at crazy hours.
Solution: Prioritize your reviews by checking the essential when a complete review is not possible (see the review checklist below). When you see yourself not being able to properly review code, try to change the timing, deploy hours.
3: Distribute/Delegate
Make sure reviews don’t fall on the same person. I’ve seen companies centralizing code reviews on one person. It makes for:
- Overworked people
- Slower deploy times
- Bad reviews (reduced attention span)
Problem: But my CTO is micromanaging the code reviews to make sure everything is alright
Problem 2: But I don’t trust my team to deploy code without my eyes on it.
Solution: Teach a man to fish. Also, make for your (or your boss's) review as an additional safeguard but not an essential blocking one.
4: Shared Code guidelines
Before enforcing a best practice, one can always make sure everyone is on board with it. Take time to view what guidelines are important. Always hear when developers want to add to the guidelines. Good people always look for ways to improve their craft. Good developers always want to stay updated.
For different programming languages, you have different coding guidelines.
If you don’t yet have a coding guideline, here’s a list of them by programming language to start the conversation:
- Ruby
- Community-driven
- Github’s style
- SO Answer
- Rubydoc
- Javascript
- Crockford’s guide
- Felix’s Node style
- Mozilla
- Idiomatic.js
- Python
- Guido’s PEP8
- python-guide.org
- PHP
- Pear
- PHP-FIG
- PSR-0: Autoloader Standard
- PSR-1: Basic Coding Standard
- PSR-2: Coding Style Guide
Taking community-driven standards (or at least basing your company’s code style in it) it’s a great idea since it ease’s new developer onboarding and better community help.
5: Create a checklist upon review
Having a checklist to review code greatly improves the efficiency of the process. Sometimes in the middle of a review, we identify an issue and we remember we haven’t been really paying attention to that specific problem previously (which then leaves you with the uncertainty of an incomplete job).
Ideally, one should have every important aspect in one’s head (since for N lines of code, checking each one for N code rules is O(N2) — absurd of course but you get the point).
Also, the inverse of having coding guidelines (point 4) is enforcing them.
Problem: There are a lot of engineering issues that don’t fit into the checklist.
Solution: Definitely right. These can be addressed afterward in a team/one-to-one short meeting. Overall, this is a learning process, and is much more important to start doing some early and learn from what is missing or being left out.
Here’s a small suggestion of a code review checklist to start the conversation in your team:
- Is the code style according to our own?
- Is this code according to the best practices we/community defined?
- Is this code problematic/inefficient/error-prone/not clear/previously proved bad/not compatible with the architecture
- DRY/SRP/KISS/YAGNI/Smell
Here are other checklists you might find interesting as well: 1, 2, 3, 4, 5, 6
6: Use collaborative review tools
Doing Git (or repository-based) reviews only (by looking at logs, diffs, and commits) is time-consuming. One should look for a tool that allows you to:
- Quickly review what changed
- Quickly be able to communicate with people
- Decide to approve (or not) the commit
By concentrating these three aspects into one tool, you can be more efficient at the underlying actions we need to execute a code review.
If you’re choosing a tool, choose one that you can get your whole team on board. Github, Bitbucket, and GitLab both have pull requests which represent a workflow with built-in code reviews.
7: Do a little, even if no time
Problem: It’s Thursday and you want to deploy some features into production (because you don’t deploy code on Fridays). However, it’s been a specially tough week and so you haven’t really been paying attention to how code evolved. Before you know it you have 200 commits to review.
Solution: Prioritize your checks and don’t strain yourself. Only check for major issues. Code style can be checked later or automatically through linting afterward.
Knowing the history of the project (and the problems you’ve had), you also have a sense of the more important aspects to look for.
The important part is sharing ownership and sharing knowledge.
8: Use short-circuiting
When time is of the essence and reviewing every line written is not an option, take an incremental approach.
When you’ve found a possible buffer overflow or a function call you know it will blow up in production, it shouldn’t really matter to review code style.
In your code review checks, you can insert short-circuiting elements: if a commit has a big enough problem that needs to have further development, one can ignore smaller less important problems for a future review of that code when it is corrected.
9: Helpful commits
Concise and to the point commit messages with code that is commented really narrows the focus of the review. The code reviewer will have a much easier job if you talk about the implemented feature and the design decisions behind it. Helping your team members is a good way for them to see how valuable a proper commit is. Take a look at how the Linux kernel guides their own submission: https://www.kernel.org/doc/Documentation/SubmittingPatches.
10: Add automated code review tools to your workflow
A final and great way to reduce the time it takes to review code is by reducing the number of aspects of your checklist that you need to pay attention to.
By adding automated code review tools you can automate some aspects of your code reviews such as code style, best practices, and common issues. This frees you to only check for what matters.
Automation lets you be better and save time. Combined with the aspects on top, you’re on your way to make sure you don’t spend more time than you have on code reviews.
Because you care and because you always want to be better, automation is a great way to optimize your review workflow process. Go ahead and do a quick search on Google for automated code reviews and see who better fits your workflow. You'll find Codacy on your Google search and we hope you like what we do.