The review process should be made as easy and stress free as possible so that developers and testers are fully engaged with the right mindset and attitude. It should not be seen as a frustrating step or burden otherwise the aim would be defeated. To facilitate the code review process, trivial and repetitive tasks should be automated. Every major programming language has its set of code quality tools that can help achieve this. In PHP for instance, this include PHPCS, PHPMD, CS Fixer, Scrutinizer, PHPUnit and Behat. These tools help the reviewer focus more on the details, by address and fixing common issues either locally within the development environment or remotely via pre-commit hooks prior to the review process. In as much as these tools are useful, it is worth remembering that they are just tools to assist in the process and not to replace the developers or tech leads. Sometimes tools give false positives which needs to be reviewed by a human being.
It is also important that there are established coding standards and agreed guidelines. Coding standards should, as much as possible, be encapsulated in automated processes and scripts – coding standards should be non-negotiable. Guidelines on the other hand, are meant to guide and not necessarily authoritative.
- Guidelines should be concise, readable and include best practices
- Guidelines should be agreed and reviewed from time to time.
- Guidelines should be easily accessible to help new starters and refresh the memories of others.
- Standards and guidelines should be relevant, up-to-date and widely accepted within the wider software development community
- Developers should be made aware of the coding standards and guidelines, and notified when they are updated
The approach and attitude of the reviewer is crucial in achieving the desired outcome. The reviewer should have a sense of responsibility of what is being reviewed, ensuring that the software released to production is of the required standard. When carrying out code review, the reviewer should be:
- Cordial and professional at all times. Profane, insulting or condescending language should be avoided.
- The tone of voice when writing should be cordial and respectful
- Good work and approaches should be acknowledge during the review and not only identify defects. Praise publicly, correct privately.
- Always put a face on the code being reviewed with the awareness that there is a human being on the receiving end of your comments.
- Consult standards and guidelines when necessary
- Consult superiors and confirm approaches if in doubt, speak with the code author for clarity when needed
- Non-critical issues could be over looked if there are differences in opinions, and if necessary, rectified in subsequent iterations when the differences have been addressed and clarity established.
- When there are differences of opinion in approaches that do not necessarily go against coding standards or guidelines, the benefit of the doubt should be given to the developer.
- Set realistic standards based on the initial quality of a legacy codebase, progressively raise the standards as the quality of the code gets better over time.
- Give helpful feedback and suggestions, don’t just identify defects without at least suggesting a possible alternative. Do not insult the intelligence of the author in the process.
- Code reviews should be done in a timely manner. Having reviews take too long is equally as good as not having code reviews at all.
- Regular workshops, training sessions and meetings should be organised from time to time to discuss and address commons issues identified during the code review process.
The number of reviewers involved in a particular code review varies depending on the size of the development team. In my opinion, no more that two reviewers should be involved in a code review. In most cases, one reviewer is sufficient – to many cooks spoil the broth.
Above all, it is important to know and understand when to stop writing and start talking. When there are more than two replies to an initial review comment and a consensus it not reached, it is time to stop adding comments and have an informal discussion about the issue in question instead of debating. The developer is more important than the code being reviewed. Always, aim to maintain a healthy development environment putting individual egos aside and resolving differences in opinions cordially and quickly.