Monday, 14 March 2016

Making code reviews part of your team culture


Code reviews are like unit testing, as developers, we really want to do them, but not every team is able to introduce them successfully. This is a post around a presentation I recently gave at What Do You Know Sydney about how to make code reviews part of your development team culture.

I want to share how code reviews in one of the teams NewsCorp are no longer a manual chore, but are a part of the team culture and are treated in the same way as scoping, development and testing.

I've worked with amazing development leads over my career who dedicate so many hours each and every night code reviewing and providing great feedback on the code developers in the team have developed. Although this works and adds huge value in ensuring the highest level of code, it is not a scalable solution.

What is a code review?

“Using your experience as a developer to review and assume joint ownership of that code”

What do code reviews involve?

  • Checking out the code locally and ensuring it runs as expected.
  • Making sure the code complies to the teams syntax standards.
  • Running the unit tests (and ensuring there are unit tests).
  • Making sure the code architecture & design patterns being used are the right ones.
  • Being 100% certain there are no security vulnerabilities in the code.
  • ...Every team will have it's own interpretation of what a code review entails...

Why do code reviews fail within some teams?

  • It's an ad-hoc process.
  • Developers don't understand what a code review should involve.
  • There is not enough time allocated to code reviews.
  • When only the leads or senior developers are doing the code reviews.
  • Most importantly, when there are no surrounding processes in place to encourage code reviews.

How do you make code reviews part of your development team culture?


Make it part of your task estimation

  • Unit testing is usually around 20% of the development time (on average), we have found that code reviews are around 5% of the development time. If a feature takes a 5 day week to develop, usually half a day is enough to code review this newly developed feature. 
  • Remember to allow some time to implement development feedback from the code review.


Utilize Pull Requests (and enforce approvals before merging)

In their simplest form, pull requests are a mechanism for a developer to notify team members that they have completed a feature. In the open source community this is normally how new changes are submitted to core contributors to review and implement into the core open source platform. Within your team ensuring that at least one developer must code review and approve a Pull Request before it is merged is a great step to encouraging code reviews within a team.

Use a great tool like Stash for code reviews

  • Stash is a great tool from Atlassian that makes code reviews simple.
  • You can configure stash to enforce an approver for every code review before it can be merged. 
  • You can actually take it a step further and not allow developers to merge code (only allow testers to merge Pull Requests, this allows the testing team to control the flow of code that goes into UAT and production environments). 
  • Stash has an awesome UI to make comments on code reviews easy and to even allow adding tasks to Pull Requests that have to be completed before merging.

Add it to your Agile board

  • Don't hide the code review process - keep it out in the open and display it on your Agile board.
  • This encourages the wider business to accept code reviews as part of the day-to-day.
  • If a tasks fails code review it moves back into development.


Use code reviews as a knowledge sharing tool

This is the single greatest value code reviews have given the team. I often hear in development teams that this developer knows this, and that developer knows that. When your team embraces code reviews, you obviously still have people experienced more in different areas of the code, but at least 2 developers understand the same area of the code. This is because 1 developer has coded it, and at least 1 other developer has reviewed it - they have checked it out locally, ran the unit tests, understood the code and gave their tick of approval for it to be released.

How can you introduce code reviews in your team?

  • Don't ask permission just do it!
    As developers it is our responsibility to own building stable, secure and scalable code. If we need to spend time unit testing and code reviewing our code, then that is our responsibility to do so (and fight for it).
  • Use a great tool to encourage code reviews like Stash.
  • Use Pull Requests within your branching strategy and enforce approvals.
  • Don’t have exemptions - it's all or nothing!!
  • Have a reward each sprint or month for the person who does the most amount of code reviews
  • Have a token for code review immunity if you find some developers are getting too many code reviews.
    We use a Jedi robe, if you are the one wearing the Jedi robe (of which there is only 1), then no person can assign you a code review until you take off the robe.
  • Most importantly, be an honest code reviewer.
    Don't just approve a Pull Request, every constructive piece of feedback you add, is making your whole team stronger and better developers.

The team I am referring to in this presentation has code reviewed  over 172,000 lines of code in the last 12 months which in my books is an impressive statistic.


0 comments:

Post a Comment