Code reviews are one of the most useful parts of the development lifecycle. There are many different approaches to code reviews, and figuring out best practice for yourself and your company can be hard. You have to do what works for your scenario, but there are certain things that all of us should be doing as part of code review.
In a code review, a developer passes a finished piece of code – be that a new feature, bug fix, or change to an existing feature – to another developer to be reviewed, before it is considered ‘done’. This process has many benefits for both the developer and the company.
The main reason to perform code review is to ensure that code quality is maintained. There is never a single solution to any problem, and by having at least two developers review any piece of code, we are more likely to create the optimal solution. Having a second set of eyes will often reveal things that the original developer did not think of, but also prevents laziness and stops developers taking shortcuts in the code in order to complete a job more quickly. Poorly written code will cost you down the line.
Code reviews also add a QA test layer to the development process. Every developer should test the code they write, both manually and with automated tests, however it is easy to miss use-cases when you are the one writing the functionality. Getting lost in what the code should do, rather than what it should not. Having another developer both read through your code and test it (manually and by running automated tests) will help confirm that mistakes were not made and that the code is bug free. Unfortunately, manual testing is often skipped during code review, and because of its name people often assume that code review means simply reading through the code. While this is useful, it does not provide a complete picture of what is happening.
The github pull request feature provides the perfect interface for delivering a code review, allowing you to comment on individual lines of code, or on the entire change. Once a reviewer has completed their code review, they can then approve the pull request, request changes or just comment without either approving or rejecting.
Due to the nature of the code review process, only constructive feedback is given. While there is no reason not to provide positive feedback, there is no benefit to the code in doing so, so it is usually seen as a waste of time. Positive feedback is no feedback. As a result of this, it is not uncommon when implementing a new code review process for developers to feel disheartened or victimised by their code reviewer. To prevent this, when implementing a code review process (or hiring a new developer), all developers should be made aware of the expectation of the process. While this is not a complete solution, after you have been through the review process a few times, it becomes normal. The aim is to foster an environment where developers strive to get their code through code review without any comments at all.
When implementing the code review process, it is a good idea to start out with a checklist of things to look out for. This ensures that all your developers are performing the same checks. While it should not be necessary to check code formatting and coding standards, provided your developers are using correctly configured linting tools, it is often the first item on a code review checklist. Other items may include: manually testing functionality, automated tests passing, automated tests covering all use and error cases, no known security vulnerabilities, no obvious poor performing or unnecessary code, cross browser testing. You will need to design a checklist that works for your team and your company’s requirements. Once your team has become used to delivering consistent code reviews, you will be able to phase out the checklist, although to ensure consistency, it can be a good idea to continue to use it for new developers joining your team.