Code Reviews
In code reviews, programmers systematically check each other's code for errors, irregular formatting, or inconsistencies with system requirements that may lead to bigger problems during integration. Code reviews provide several key benefits, including:
- Improves code quality
- Expose issues in logic
- Staying consistent
- Supports knowledge transfer
- Improves performance
- Helps in Mentoring newer engineers
- Improves documentation
- Makes testing better
Code Review Guidelines
We have compiled a code review guideline to help teams maintain a high code standard and cohesiveness throughout their codebase.
Readability and Formatting
The code should be readable and comprehensive. Comments should be clear, useful, and explain why instead of what. The style guide of project should be followed e.g. linting, formatting, types, and other standards.
- Is the code readable?
- Are components, functions, and styles sectioned into files with appropriate extension, i.e. tsx | jsx | js (Separation of concerns)
- Is the code linted? (should be automated)
- Is the code commented?
- Are project's naming conventions followed?
- Is the code cleaned up (console.log, todos comments, unused variables, commented code etc.)? (should be automated)
Implementation
The code in review should be fulfilling the intended while also ensuring that it does not cause side-effects or compromise the system performance and functionality.
- Does the implemented functionality fulfill the subject requirement?
- Does the code not pose complexity for peer developers?
- Is variable scope properly handled? (const/let/var)
- Do functional components not have any states or logic of their own? (Composition)
- Is destructuring used to unpack values?
- Are proper types defined? (Typescript)
- The change does not add unwanted compile-time or run-time dependencies?
- The code has been checked for code duplication?
UI Implementation
- Are class names meaningful and follow conventions?
- Is unit consistent across all style files?
- Is inline style avoided?
- Is
!important
avoided? - Is semantic markup used?
Error Handling and Logging
- Are exceptions handled?
- Is a proper message thrown for an exception/error?
- Are inputs validated before processing? (e.g. optional chaining)
- Are edge values validated?
- Are cases like API call fail, no/slow network connection, permission denied, etc. handled?
Performance
- Is re-rendering controlled?
- Are relevant dependencies listed?
- Is memoization implemented where applicable?
- Is it sure that the states don't update in a loop?
- Are promises/async functions implemented correctly?
- Are cleanup functions written?
- Is the code secure and there are no memory leaks?
- Are there any API requests that can be done less frequently?
- Are the libraries used, if any, reliable?
Reusability
SOLID principles should be used efficiently, making the code reusable and scalable.
- Is the reusabilty principle followed where applicable?
- Are UI components reusable?
Testing
Tests should be added, as appropriate, in the same changelist as the code. Tests should be correct, sensible, and useful.
- Do the tests covers user flows?
- Do the tests cover edge cases?
- Any complex computation logic function is covered with unit tests?
- Have any test become obsolete after the changes? If yes, have they been removed?
Git
- Are commits messages small and understandable?
- Is
.gitignore
provided? No files/folders like node_modules, dist, IDE files are committed? (should be automated)
Accessibility
The UI built should support accessibility. (See WCAG checklists and its importance)
- Are Web accessibility standards added?