The main goal of code review is to validate that implemented functionality meets the requirements in an optimal way from performance, architectural, and security perspectives.
In addition, code review is intended to make sure code meets Adobe Commerce development best practices, is easy to understand and maintain, and follows coding standards. Most coding standards should be checked automatically by appropriate tools.
Recommended code review process
At a high level, the code review process should consist of the following steps:
Checkout feature branch on local development environment.
If it’s been awhile since code was submitted for review, merge the latest changes from the target branch (usually QA branch), and push updates to the remote feature branch (if any).
Do a high-level review to validate that the code implements all requirements. If there are major discrepancies, contact the developer for clarification.
Running the code is optional, but if you have doubts that the code works as expected or if it facilitates the efficiency of the process, test that the implemented functionality works as expected for main usage scenarios. If there are major issues, stop the review process, and reopen the ticket with appropriate comments.
Do a thorough review to validate that the code implements all requirements. If there are any issues, add appropriate comments to the pull request and reopen the ticket.
If everything looks good, merge the pull request (or pass it to next code review level if one has been established).
Also, consider the following points when when implementing code review processes:
Code review is a primary tool for communication and knowledge transfer within a team. Even if a pull request doesn’t contain major issues and implements the required business logic, you can use it as an opportunity to suggest more improvements after it’s been merged.
On average, code review shouldn’t take more than 10% to 20% of development effort. That’s assuming that the development team consists of senior engineers that work well together. If code review takes longer, it can affect project budget and timeline.
Address issues with excessive effort required for code reviews by identifying the root cause. Usually, it’s because either requirements are poorly translated into development tickets (due to communication issues, poor user stories) or it’s a coaching issue. In the first case, the recommended mitigation is to make sure that tickets have enough information for team members to efficiently deliver the requirements. In the second case, you might need to adjust the team structure to include more senior engineers or get approval outside mentoring and coaching.
Does the code handle customer data or emails? Pay special attention.
If this code can be executed in a loop, can it leak customer data from one loop cycle to another?
Indicators of a risk are imports, cron jobs, transactional emails, and batch queue handlers.
Ensure the isolation of user data in loops. Adobe advises using factories or repositories to create models in the loop cycle, which are not accessible outside of the loop.
Reviews for mentoring are done manually.
Look for places to share knowledge with the goal of educating the team.
If your comment is purely educational, but not critical to meeting the standards, indicate that it’s not mandatory for the author to resolve it.
If you see something nice, tell the developer, especially when they addressed one of your comments in a great way. Code reviews often focus on mistakes, but they should offer encouragement and appreciation for good practices as well. It’s sometimes even more valuable, in terms of mentoring, to tell a developer what they did right than to tell them what they did wrong.
Developers can use automation to review DI compilation, database schema, composer validation, and compliance with the coding standards:
DI compilation—Run the following CLI commands to see if the code can be compiled without any issues.