In this post, I'm going to talk about how we have adapted code reviews to fit into our Agile processes.
Code reviews are good for the developer, not the project.
While traditional code reviews are good for the progression of the coder (and hence the next project, and company), they inherently do not impact or improve the "just finished" project.
Traditional code reviews normally happen at the end of the development life-cycle. This means that any issues that are flagged with code/implementation are not actionable. The project has already finished.
Code and Tell
A while ago we implemented a weekly informal code review which we call "Code and Tell". Basically, every week our developers pair up and review each others code. We rotate the pairs every week to allow each developer to be familiar with all the projects in the company. Sometimes, we get in groups of 3 if someone has a lot of code to show. It's also a chance for everyone to show off something in their code which they are proud of.
What are the benefits of weekly code reviews?
- Any implementation issues are dealt with on a weekly basis. This speeds up development.
- Any questions/help required can be addressed on a weekly basis. This also speeds up development.
- Code standards/styles are addressed more quickly. Consistent code across the company means other developers will have an easy (faster) time reading the code ,and taking over the project.
- In an agile work environment it is exceptionally important to keep track of code, as things may actually change on a weekly basis. Weekly reviews allows changes to be reviewed immediately, not at the end of development.
What are the benefits of developers reviewing each other?
- Projects become familiar to other developers who are not on necessarily on the current project team. This means that they can pick up the project at any point without the need of a "handover".
- Not as intimidating as formal reviews. They're very relaxed and more about helping, rather than criticising.
- Easy to get your boss to agree to internal reviews, rather than "resourcing concerns" to do with pair programming. That is, pair programming from a resourcing point of view may be difficult to implement due to company/time constraints.
- Developers not only see how code was implemented, but can also ask questions about "why", which furthers their understanding of the codebase.
- In essence, what we are actually doing is short bursts of pair programming.
A quick note about handovers... Normally they last for about 1 hour. But how much of a project can a person remember after 1 hour code handover? And how useful was that handover in the long run? Moreover, handover notes are sometimes just as useless, because a developer must read and try to understand a project after it has been finished.
When to review and how long?
We review every Thursday morning, and we try to keep reviews to 5mins (yes 5mins!). 10mins is the maximum.
Why Thursday morning?
It's the perfect time of the week. It gives developers 3 days of solid coding. But it also gives them 1.5 days to action anything that was flagged during the review.
Why only 5-10 mins?
It really only takes 5-10 minutes to show off some code and have it critiqued. If you think about it, 5mins per week over an 8 week project is 40mins worth of reviews. Doing 5mins per week from the very start means that people get quick glimpses of the project at every stage. Obviously sometimes it goes over 5mins but we always try to keep it short.
Traditional code reviews, while useful for the individual developer's progression, are not useful for the project. By having developers review each other on a weekly basis, not only do projects gain better structure and coding standards, but the production time is improved. Moreover, by rotating reviewers, each developer in the company gains enough knowledge about all the projects, which enables them to quickly pick them up without the need for a handover.