The development workflow should include code reviews particularly as code reviews continue to cement as the foundation by which developers share code knowledge and ensure best decisions, the need to formalize this process increases.
This is important for many reasons:
- Consistency: you want to make sure you’re on the lookout for the same things in each PR
- Agreement: getting everyone on the same boat is hugely important as it increases the likeliness of people respecting design decisions, style, best practices.. etc
- Speed: if there is a process in place, you are more likely to spend less time with adhoc discussions
If you find yourself and your team reviewing code informally, these are a few steps you can take to push forward the discussion about the need to make it an unavoidable process.
1: Make it a habit
A great book on the habits describes how they form and how our brain is wired to execute them despite us not remembering them. This is due to the primal nature of habits. There are excellent examples of companies tapping into the underlying habits of consumers to sell more.
The book describes their creation as a need for three things:
- A Cue, a signal by which the routine will quick in;
- The actually Routine to execute;
- A Reward to incentivize this behavior
How to form habits: cue triggers a routine that creates a reward
An example of this (in the book — which is an excellent read) is how Pepsodent started selling more toothpaste. They understood their cue was when people started feeling a ‘film’ around their teeth, so they made their advertising around that: “You’ll feel a film — that’s what makes your teeth look ‘off color’ and invites decay.”
To make code review a habit in your team, you can follow these learnings to your advantage.
If you apply this to code reviews, we can have several examples:
- Cue: a developer asks for a code review on PR#1 OR a notification appears on Slack announcing a new pull request ready for consumption
- Routine: the code review process itself
- Reward: to write a ‘LGTM’ or a funny engaging GIF; to understand and be thanked by the rest of the team; to get bragging rights because I’ve pushed the project forward (a feeling of achievement or ‘I got shit done’).
There are many ways to push for this. We actually have a lot of ideas in Codacy around this topic so stay tuned.
2: Priority queue
You have 10 open PRs waiting to be reviewed. Your team mates have asked you recurringly to review their PRs. All of a sudden a urgent PR comes in with a feature for a client that business has asked you to approve quickly. You do it but you are left in a weird position with your team members.
Enter priority queues. PRs are normally seen as a FIFO when in fact they aren’t. Many things happen from the time we start developing a feature from the time we push it upstream. Maybe priorities changed, maybe product changed.
This is why having a priority queue is important. You weight the importance/urgency of each PR with a scale such as Bug fix, Urgent, Critical, ShowStopper, etc.
Most of us already have this priority in our ticketing systems so there’s nothing new here. The best way is to integrate this priority in your PRs and have this information dictate the priority of review.
3: Centralize or Power to the quorum
When is a PR ready to go into staging/production? Who makes that call?
We’ve observed that, from different other scenarios, there are two main decision strategies:
- Centralized decision: one person (or a very small team of people) decides when something is ready to merge
- Quorum based decision: a minimum set of N approves from the team is required to pass to the next stage.
Centralized decision forces one single person knowing the whole system. There tends to be less disagreement as a leader decides. Speed of merge tends to be faster on decision to merge but slower on coverage of PRs.
There’s also the ‘single point of failure’ problem. If the leader is “run over by a car” or (more realistically) on vacation the show must go on.
Quorum based decision forces a team to know what is being merged and to share responsibility on what is entering the code base.
Speed tends to be faster covering pull requests but there’s always the risk of entering in infinite discussions about certain topics.
Both have pros and cons. The most important is that you use one of these two.
Ultimately you can fuse the two. But since in the end the purpose is to actually ship software and not enter into advanced political science thesis, I believe you know what will work best for your team.
4: Find ways out of disagreement
Disagreement and conflict is not only natural but required as many design decisions need to be viewed from different points of view. However, there needs to be a point by which a decision is made. A moderator is a good role to take care of this situation.
If two (or more) people enter in disagreement
- Wait for them to agree OR
- Step in and nudge
5: Review checklists
Before enforcing a best practice, one should 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
- Scala
- Official Scala Style Guide
- Effective Scala
- Apache Spark Style Guide
- Paypal 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 on boarding and better community help.
6: Integrate and Automate
The best way to really formalize a code review process is to reduce the dependency of humans.
By reducing the number of tasks developers actually do, you are in a better position to make sure a process is executed.
Integrating static analysis and tools like Codacy in your workflow allows you to reduce the burden of reviewing code.
At least 20% of your effort is removed so that’s an important gain.
Furthermore, if you integrate services (like Github, CI and Codacy -that ensures most of your checklist) to your workflow, you are making sure the process is unavoidable. It becomes hard for someone to disregard the review process making it formal and enforced.
You should be relatively flexible on any of these. But you should be stern and resolute regarding formalizing code reviews.
Edit: We just published an ebook: “The Ultimate Guide to Code Review” based on a survey of 680+ developers. Enjoy!
About Codacy
Codacy is used by thousands of developers to analyze billions of lines of code every day!
Getting started is easy – and free! Just use your GitHub, Bitbucket or Google account to sign up.
