reference:http://blog.codacy.com/top-10-faster-code-reviews/
This is a blog post of our Code Reading Wednesdays from Codacy (http://www.codacy.com): we make code reviews easier and automatic.
Follow the discussion on Reddit and Hacker News
How frequent is it for you to be reviewing code at 3am?
When code reviewing, do you find yourself thinking: "I mentioned this before.. We should have some sort of process".
We're about to reach 2000 developers using Codacy and I've learned a ton from them along the way.
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.
I've gathered some aspects from people who are doing it right.
These are small hints that I've seen from our users that highly help so you don't have to be alone at 5am 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, 5), 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 on reviewing your code, they will most likely follow.
2: Time boxing
Code reviewing tends to be secundary 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 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 check list below). When you see yourself not being able to properly review code, try to change timing, deploy hours.
3: Distribute/Delegate
Make sure reviews don't fall on the same person.
I've seen companies centralising 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
Problem2: 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) review as an additional safe guard but not a 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
-
Javascript
-
Python
-
PHP
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 on boarding 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 afterwards 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 from me 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 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 and Bitbucket both have pull requests which represent a workflow with built in code reviews.
Going forward, you have brilliant tools from the smart guys from SmartBear and also Crucible from Atlassian
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 afterwards.
Knowing the history of the project (and the problems you've had), you also have a sense to 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: Automate with Codacy
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.
Codacy is a tool we built to automate some aspects of your code reviews. Codacy checks for code style, best practices and common errors. This frees you to only check for what matters.
Furthermore, you can always refer back to Codacy whenever you find an issue.
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 to doing code reviews.
Because you care and because you always want to be better, automation is a great way to optimize your review workflow process.
Conclusion
I've passed through the experience of our users when reviewing code for their companies.
Overall, this practice will always lead to better code, shared knowledge and bigger consensus. However, improving the process is something we can greatly benefit from.