Effective Code Review
- 1. Effective Code Review 21 / 01 / 2020
- 2. What is code review? Code review is systematic examination of computer source code. It is intended to find mistakes overlooked in the initial development phase, improving the overall quality of software.
- 3. Code Review is the most important tool we have
- 4. Code Review Goals Find bugs Enforce standards Suggest improvements Share knowledge Learn
- 5. Code Review Goals Enforce standards Scale Maintainability Performance Observable Tests Clean Code Consistency Code Reuse Readability Coding Style
- 6. Code reviewee You job is to convince the reviewer on your changes Be prepared Know why your changes are necessary Understand where they fit in the bigger picture Think about the potential risks Be ready to defend your work Not everything that code reviewer says must be done Always be convinced before accepting suggested changes Go over code review checklist Do yourself a code review prior to real one
- 7. Code Review Checklist Release 04 Understand what are the risks of this change Release plan: what should be deployed and in what order Are there any related projects that this might break? PM/Design are aware and approved this to be released? Tests 03 All tests passed: unit / integration / e2e / lint New tests added for new code None relevant tests were deleted Changes were tested in local / CI environment Code 01 Code is working No temporary code No redundant comments Deleted not used code Pull Request 02 PR should be opened Commits are complete: commit description / each is working / correct order Clear Description Branch is rebased on master
- 8. Code Reviewer Your job is to guard the master You must be convinced the suggested changes are an improvement Ask for context and understand the reasons for this change Deliver knowledge that relates to the topic Opportunity to mentor and teach about related areas Attach links to article / posts / stack overflow for further reading / learning Discuss what can go wrong Is this code introduce breaking changes? Talk about how this change relates to the general architecture / design Reviewer should mind code smells
- 9. General Guidelines & Tips Best to do the first code review iteration live Reviewee give context Opportunity for broader discussions Inline changes Style suggestion: Read the tests first The one who started code review needs to finish it Every ramped-up team member can do code review Merge (and deploy) right after code review is done Otherwise, rebase and conflicts may be needed, wasting time doing code review again Always write down your comments on PR So you wouldnt forget them Use resolve / / done to mark completed comments
- 10. General Guidelines & Tips Always be convinced If not, go ask other peers, or manager to settle Read every character in PR The responsibility for bad code getting to master is on the code reviewer Once a code is in master - it is treated as correct Even if you dont agree with the style, or with the solution Please close stale PRs Try to leverage linter instead of obsessing about styling notes Always be nice
- 11. Thanks!
Editor's Notes
- #9: a code smell is a hint that something has gone wrong somewhere in your code. Use the smell to track down the problem.