Everyone of us does code reviews daily, weekly, or at least monthly, however, what strategy do you use to review code? I hope you're not jumping on a piece of code and start reviewing. Writing code is an art, similarly reviewing it.
Background
Even after writing and reviewing the code in different languages for more than a decade, I still stumble when someone asks me how I perform code review. Hence, I wanted to create a post so that someone like me take it as a baseline and perform an effective code review.
How to Review Code?
1. Intention & Comment
The most important and above all is the intention behind any work matters, the same applies when it comes to code review (I would say more than anything else). If you intend to foster a positive culture for peer code review, then you'll act accordingly but if you wanted to show off your great knowledge while reviewing the code then you're going to do a bad job.
Don'ts
- Never try to be a hero when reviewing the code, or else you'll become a bottleneck for everyone and making others' life difficult, and slow the team to deliver new features.
- Don't use harsh words when commenting on a code review request, remember you're part of the same team or organization. And we succeed as a team not as individuals.
- Please don't comment for the sake of commenting, some code changes are awesome. And if you still can't resist then just comment LGTM (Looks good to me)๐.
- Don't try to impose your coding style preference on the author. Programming is a beautiful art, and itโll be alive until different developers write code differently.
- Don't rubber stamp any code change, remember tomorrow you may be working on the same lines of code.
Dos
- Always provide constructive feedback, making mistakes is not a crime. And that's the reason we have peer review, else we'll be pushing code to production without any review.
- Always keep the conversation open, when commenting on a code review the conversation should be bidirectional, always suggest code change reasonably e.g. Could this be done like XYZ way, or Just curious to know the reason behind this. You can always learn something new everyday.
- Always provide the reason behind any comment, and justify your stance. Think would you incorporate the same comment?
- Most importantly, keep your ego out of code review, just keep it aside.
- Be precise about what needs to be improved, elaborating on why
- Review the code as you're going to work next on it
- Add comments like "Good to have" or "Nit" for polishing the code
2. Standards
Once your intentions are good, you're all set for the next up, what to look for when reviewing the code?๐ค
- Readability: If I can't read the code very well then probably I can't understand it.
- Linting and Styling/Formatting - I want to leave linting and code styling to be a part of the automation pipeline, that's something the automation pipeline should take care of or part of the project. However, the team should agree on which plugins to use like Spotless, husky, etc. Team Agreement is a best place where such agreement could be documented. If the pipeline is not capable of performing linting or formatting checks then it can also be achieved via Git pre-hooks. Below are a few posts on pre-hooks
- Complexity: Complexity shouldn't be confused with non-familiarity. If a code looks complex because you're not familiar with it, then that is not a complex code (it could be because you're not familiar with new syntax, if that's the case you upgrade yourself). If you're proficient in the language you're reviewing code and it takes time to understand the logic then it needs some level of simplicity
- Naming Convention: Though naming a variable is hard, we need to make every effort to choose the least bad name.
- Testing: There should not be any compromise when it comes to unit, integration, and end-to-end testing. However, code coverage, and test result evidence should be part of the automation pipeline. The automation pipeline reduces the burden on the reviewer, but test code should also be written as production code.
- Code Duplication: Duplication should be caught by tools like SonarQube during the pipeline. However, you still need to configure the code quality to a higher level, however, the code quality level depends upon the organization or team's direction. Nevertheless, look for any duplication.
3. How to Approach Review
Start with the big picture, does code change speak about the update per the code change description? Next is to look through the important code logic updates and lastly check out the other files.
Not all code can be just reviewed over the source code control interface like GitHub, BitBucket, etc. Sometimes we need to run coding locally to review, this is especially true for UI code changes.
If you have reached here, then I made a satisfactory effort to keep you reading. Please be kind enough to leave any comments or ask for any corrections.
References
- https://google.github.io/eng-practices/review/reviewer/
- https://smartbear.com/learn/code-review/best-practices-for-peer-code-review/
- https://www.atlassian.com/blog/add-ons/code-review-best-practices
My Other Blogs:
- Micro-frontend Decision Framework
- How to Reduce Page Loading Time - Part 1
- Avoid Spring RestTemplate Default Implementation to Prevent Performance Impact
- My firsthand experience with web component - learnings and limitations
- Load external font with web component
- Test SOAP Web Service using Postman Tool
Top comments (1)
Great post ๐ฏ