This guest post is brought to you by Zando.co.za engineering team.
Zando.co.za is South Africa’s largest online fashion retailer.
We are part of the Africa Internet Group stable of companies. A little over a year ago, we implemented a “code review culture” in our team (although at the time we didn’t realise it), to improve the way we develop and maintain our eCommerce platform.
At Zando, we are constantly improving the speed, stability and features of our website – and (as every eCommerce venture will tell you) defects in production directly correlate with a decrease in revenue. Given this fact, we work really hard to keep our website defect-free and continually optimised – and we use code reviews to accomplish this.
We at Zando are big proponents of JetBrains tools. Our entire team uses PhpStorm for our day-to-day coding (some of us have been using it for years), and two months ago we started using Upsource to review every single line of code that makes it into production.
We have been using Upsource 2.0 since the day it was released on 20 May 2015, and to me personally – this is the tool I’ve been dreaming of for code reviews since we implemented our code review culture. We experimented with the initial version of Upsource some time ago, but at the time it wasn’t quite ready for our particular way of working – but now as of 2.0, it fits our process seamlessly, and has improved our code reviews immensely.
Finding the Right Tool
Initially, like most teams, we started using GitHub to manage both our Git repositories and our code reviews. Again, like most teams we’ve spoken to, we’ve found GitHub to be severely lacking in its code review functionality; this is ok though – GitHub does one thing and does it extremely well, and we still use GitHub to manage our Git repositories, but it fails us in code reviews in a few specific ways:
- Notification Overload: Pull Requests can be assigned to a single person, but notifications are sent team-wide, and this produces cognitive overload when our team is producing several new PRs a day, and comments are made on those PRs constantly.
- Diffs are summary, not incremental: Once a PR is initially reviewed and comments made, new commits are added – but it is not possible to only review newly-made modifications on top of what has been already “reviewed”.
- Lack of basic “review” idioms: GitHub PRs are not really designed to be “reviewed”. They are extremely basic in their configuration, and there is no concept of provisionally rejecting or accepting a review in its current state. The reviewer is left to use 👍 or 👎 which is not terribly useful for managing many developers’ worth of reviews.
- Lack of “discussion” resolution: Comment threads work well in GitHub, but again the reviewer has to use emoji to indicate whether they are satisfied with the outcome of a discussion.
GitHub is overall quite pleasant to work with for its stated purpose, but definitely not for a team with a mature code review process.
We tried Reviewable.io and liked it quite a bit actually, but it would regularly choke on larger reviews and general felt a bit sluggish since it is a completely client-side application. We also didn’t want to necessarily be locked into GitHub forever either, with great tools like GitLab becoming quite attractive.
After becoming irritated with GitHub’s shortcomings, we moved over to GitLab a few months ago.
GitLab has done really well to replicate GitHub, and makes a number of improvements on GitHub’s deficits. One big feature we found useful was the ability to prevent merges into particular branches; we use Git Flow, and it is helpful to protect the develop and master branches from unintentional (or intentional) merges without review.
Another thing that GitLab gets right is not spamming the entire team with emails after every comment, PR or merge! Just this alone made the team much happier and feel less overwhelmed with meaningless noise in their inbox.
Sadly though, as stated above, GitLab has copied GitHub a bit too closely with its unusable review process – and so we gave GitLab the boot and moved back to GitHub.
After watching the demo videos for Upsource on the day of its release, we installed it right away. As a team, we’ve come to know and trust the JetBrains brand – and were excited to experiment with the new shiny toy! Installation was extremely simple, and we were up and running very quickly.
JetBrains put a lot of thought into 2.0, and it now boasts a lot of the features we’ve been waiting for since we began code reviewing intensely:
- Incremental reviews
- Targeted notifications (less inbox overload!)
- Review Accept/Reject idiom
- Syntax highlighting for all languages
- Discussion threads with the ability to “Resolve”
- Multiple reviewers/watchers
- Ability to manage any Git repository, regardless of hosting platform (in-house, GitHub, etc)
- Repository browsing
- Comments on sections of code rather than single lines
- Upsource integration in PhpStorm 9 (wow!)
- Extremely fast UI with good memory management
- Excellent documentation
- An enthusiastic and helpful development team (I always get same-day responses to feature requests & bug reports)
- Upsource also comes with a 60 day trial license, which allowed us to thoroughly evaluate the software with our team over a fair amount of time.
Code Review Culture
Code reviews are a team tool. Like in any creative industry, we need “editors” with experience to help us identify blind-spots, areas of improvement and problematic aspects of our code – to improve our craft. Code-reviews highlight where a developer is strong, but also where they need to improve.
Code reviews not only help reduce defects and improve developers’ repertoires, but they also tend to decrease “silos” by giving complete visibility and responsibility to the team of each other’s work.
Here are some helpful tips to implementing a code review culture in your team:
- Use a development methodology like Git Flow – don’t work directly out of master!
- Separate every bugfix or feature into a self-contained, single-purpose branch
- Submit PRs for these branches when you (as the author) feel it is complete
- Encourage a culture of “peer review” like in academic circles – team up two similarly-skilled developers and have them review each other’s work (horizontal interrogation)
- Have more senior developers review larger features or bugfixes (vertical guidance), after the peer review
- Discuss, don’t dictate – when reviewing, assume that your developers think carefully about their choices and ask them why they opted for a particular approach, rather than dictate how they should do it instead
- Praise good work
- Discourage lazy or careless behaviour
- Encourage stylish and simple code
- Decide as a team on a style-guide to avoid silly arguments over tabs vs spaces, newlines, etc
- Be polite, respectful and encourage developers to find their own solutions
- Keep your PRs and reviews small, and focused. Aim to spend 10 – 15 minutes per review, and 5 minutes or so for each incremental review
Many thanks to JetBrains and the Upsource team for a great piece of software that gets things done, instead of getting in our way!
Danny Kopping is a Lead Developer at Zando.co.za.