The Value of Code Reviews in an Engineering Team

Problem

Moving to this practice is not easy and involves a change in your workflow, many teams are quite used to committing code without reviews. It would furthermore be reasonable to say some people think it adds a restriction to how they work in the following ways: huge, time-consuming reviews, sensitivity to feedback, and lack of proper communication.

Actions taken

There are different ways to approach code review. Some informal and some quite rigorous, it depending on your organization and what works for the team. At Phorest, we believe it's most important to have minimal interruption to your workflow. My personal experience is that the more process and tools put around the code review, the less context and value it has. Over the shoulder and pairing should always be the preference if you want to build a collaborative team. It also reduces noise and keeps the time to perform a code review to a minimum. This can be challenging for remote teams or people in different time zones come into play, in this case a PR is the preference.

• Pull Request  // When working on a new feature you branch from the development one and work from there, when complete you create a PR from your VCS (commonly Github or Bitbucket). You assign the PR to a team member, they can comment and view all changes from the new feature or bugfix branch and merge it into the development branch.
• Over the Shoulder //  A much more direct approach is where two team members sit together (or video call) and go through the changes. This works so fluidly that you don't even need to branch off from the development and can commit directly. It can often lead to white-boarding and paired programming sessions and can be considered a very collaborative and fast technique.
• Group/Committee Review  //  Done in a formal, time-boxed meeting in which all the developers come together to review implementation of a particular piece of functionality. This is a quite verbose and a corporation style.

Lessons learned

I have personal experience in two organizations where we were working in isolation and moved to this practice. It paid dividends within a short timeframe and helped us build great products, catch bugs and form a great team. As engineering teams grow and become more mature this is a natural progression. I have worked in organisations where PCI and security implications come into play and all code must be reviewed to pass those standards. It also factors into QA reporting and requirements. The following are key takeaways that I believe touch on the true value of code reviews:

• There should be a sense of responsibility to review your colleagues code and make sure it's optimal and are happy for it to be part of your active codebase. Reviewers should be liable for malicious, poorly tests or just plain ugly code. Commits or merges should actually include the reviewers name to keep that audit trail.
• For huge reviews, it's worth looking into how items of work are being divided up and how they could be smaller, easily reviewable, testable pieces of work.
• For communication, the key is to build a culture of open knowledge. You need to set a context for the reviewer and let them understand exactly the purpose of what's going on. For remote workers it's healthy to jump on a video call and run through the code and talk it over.
• In terms of sensitivity, a code review should never instil a sense of criticism or sensitivity for the person asking for it. The code is there to be optimised and a reviewed will only help you make it better or provide pointers. You should never make someone feel like you are judging, patronising or better than them. Higher Quality Software // Code reviews can often catch human error, bugs or misunderstandings of the system. When submitting your work for peer review you ensure it's polished and well done, leading to cleaner code. You also gain the expertise and input from colleagues which lead to better solutions, optimisations and catching fringe cases. You get better production quality in your software, fewer bugs and even a more maintainable or future-proof product. Collaborative Team // The engineering team forms a state of awareness and they become more familiar with the work each other are doing. It encourages an across the board understanding of what code style should be used, applied patterns and how automated testing should be done in specific scenarios. Individuals grow as professionals who learn and feed from this shared knowledge. I've seen and taught engineers how to approach TDD and write automated tests which gives huge value. Team members also get to know each others strengths and weaknesses (e.g. the regex expert). Definition of Done (DOD) // Along with many other Agile principles, the DOD helps us define that done means we have a shippable piece of work with the correct automated tests, test coverage and that it has undergone peer review. The DOD must include a clear understanding of how and when the feature will be shipped which should normally be caught during the planning stages. Failing to think about deployment and realizing during development can be an expensive and time-consuming mistake. The code review helps team members ensure features are complete to an appropriate manner as to the agreed DOD and meets the acceptance criteria of a user story. Reduced Bus Factor // Engineers don't like being stuck in the same areas for long periods of time, stopping the individual from learning new, cool technologies or working on other areas of the business. It could turn into a scenario where you lose that valued employee because of this. If you rely heavily on individuals for specific features, you have a huge bus factor and the goal is always to reduce this. The code review helps alleviate this problem and creates a set of people who understand important areas of the software and can chip in when needed. https://nothingventured.rocks/the-value-of-code-reviews-in-an-engineering-team-ab1482d26717