8. Code Reviews Flashcards
What can be reviewed
- application source code
- source code and documentation
- test code
- design description
- requirement specification
Why undertake code reviews
- detect defects in software
- identify refactoring opportunities for poorly structured code
- develop a shared understanding of code between developers
- share good practices
- gain knowledge about legacy items
When to code review
- periodic on a portion of the accepted code base
- on a proposed change to a codebase
- before a proposed change to a legacy item
defect detection rate formula
total defect = method defects + undetected defects
defect detection rate = defects / total defects
code review workflow
- developer initiate change proposal(merge request)
- developer nominate a reviewer
- Reviewer review change
- reviewer submit a recommendation
- developer makes changes and sends back for review
- reviewer approves
good practice for merge request
keep merge request small
- single review should have about 300 lines of code max
- possible to complete within 30mins
- break larger merge requests in smaller change packages
- incorporate review time to team workload model
Describing the change for merge requests
- give it a title
- explain the purpose of change(corrective, adaptive, perfective, preventive)
- report wider impact of change
what to automate
- block merge request that do not conform to standards
- max size
- break test
- code styles violation
- commented-out codes
- spelling mistakes - static and dynamic analysis for locating “smells”(code with bad practices)
- source code metrics
- design metrics
- test suite metrics - create merge request template to ensure consistency
static analysis tools
- design metrics
- used to identify part of system that are poorly structured - source code metric
- number of lines of code per function
- ratio of executable vs comments
- number of spelling mistakes - software process metrics
- number of changes per commit
- number of defeats discovered
questions to ask for code reviews
- purpose
- does this solve the problem
- how would I solve this problem
- has the documentation, user-guide been updated - quality assurance
- what are the typical, extreme cases and are they covered in test cases
- what are the corner cases
- are the test organised and readable
how does it handle exceptional situations - architecture
- does the change follow the existing architecture
- are there missed opportunities for reusing existing code
- are there clones - code
- do identifier follow project naming convention
- is code self-documenting
- are all comments necessary - non-functional consideration
- are performing optimisation possible
- are efficiency optimisation possible
- are security patterns followed
Google code review process
- ownership
- tree structure with explicit ownership by set of people - readability
- code styles and norms within workflow - code review
- lightweight review by author and reviewer - suggesting a review
- minimal set of reviewers suggested by tools to fulfil the requirement - code analysis result
- automated tools used to provide additional comments to support reviewers
Google Modern lightweight review flow
- creating
- author make changes, create a change request - previewing
- author preview the changes and results of the automatic code analyser. the change request is sent to one or more reviewer - commenting
- reviewer analyse code, add comments. the author responsible to take action for unresolved comments - addressing feedback
- author made changes, reply to comments
- all changes snapshotted, cab be seen by the reviewer - approving
- reviewer approve changes and mark the code review as LGTM
- changes reflected with at least a few review approvals
Google review frequency
about 3 changes/week
less than 7 changes/week for 80% of authors
abut 4 changes reviewed/week
less than 10 changes reviewed/week for 80% of reviewers
Google review speed for small and large changes
less than 1 hr for small changes
ard 5 hrs for larges changes
Good review size
more than 35% is single file modification
about 90% has less than 10 file modification
