I recall having a team lead when I was at my first job, and my revision control was showing the changes with red font color (I think it was Hg Tortoise 😄 ), and at some point, he told me,
You should see the red lines of the code that you changed as blood; they should be taken seriously. Do not commit your changes till you make sure".
I still remember those words vividly, and they have stayed with me ever since. Quality is essential when it comes to software development, and it should never be taken lightly.
Why reviewing merge requests carefully is important
Reviewing merge requests is a critical step in ensuring that the codebase remains high-quality, consistent, and reliable. Without proper review, code changes could introduce bugs (even by fixing a bug) or other issues that could lead to crashes, security vulnerabilities, or other problems. Reviewing merge requests helps catch these issues early on before they become bigger problems down the road.
It is not enough to only check the code quality
When it comes to reviewing merge requests, it is crucial to take a comprehensive approach. One purpose of reviewing is to make sure the changes are consistent with the overall codebase's architecture and style however It is not enough to only check the code quality; there are several other aspects to consider, such as functionality, performance, security, documentation, and testing. Check out the feature/fix/hotfix/... branch and try to test it in action and ensure it's working as expected based on what's described in the ticket. Ensure that the changes are consistent with the overall codebase's architecture and style.
The MR checklist is a blast
Create a checklist of all the aspects that need to be considered when reviewing a merge request. This checklist will help ensure that every aspect that we are concerned about is tested by the reviewer, even technical details.
As an example:
* [ ] By adding this feature the payment information show still work
* [ ] It MUST be reviewed by Zsolt
* [ ] Preview mode should be handled in this case
- Technical writer review items:
- [ ] Test coverage should be still more than 90%
- [ ] Cache invalidation should be fine
- [ ] Ensure a release milestone is set
Different View, Different Perspective
Ask for review from someone outside of your team/squad for some cases that you think [s]he has some insight in regard to the MR, as they can provide a fresh perspective and help identify issues that the team may have missed.
Mandatory reviewers
Make it mandatory that the merge request must be approved by someone who is working with you on the same feature, who might have some ideas around your approach, or who has a reputation for maintaining high-quality code.
Merge request template
Actually, they come in handy in order to explain what the reason was for creating this merge request. Basically, it leads you to guidelines, instructions, and a checklist that developers can follow to ensure that their merge requests are of high quality, and they should provide all of the necessary information upfront.
Conclusion
In conclusion, quality is a crucial aspect of software development, and it should never be taken lightly. Reviewing merge requests and using testing tools can help improve code quality, test coverage and identify potential issues before they become problems. By following best practices and continually striving to improve quality, we can create more stable and reliable software systems.
Top comments (0)