Benefits / Targets
- Guarantee the code's quality before QA, e.g. no bugs, best logic/algorithm, conformance to coding style, etc. wcfNote: IMO, code review originates from QA demands that every code line merged into master branch should be stable.
- Make sure every member is familiar with the whole code base during evolution, esp. in an agile team. Then there exists no critical person whose leaving will become a bottleneck.
- Knowledge sharing between team-mates.
- Code reviews mentor newer engineers.
- Help build team networking. When an author selects reviewers, they cast a wide net across the team. At Atlassian, many teams require two reviews of any code before it's checked into the code base.
Agile teams are self-organizing, with skill sets that span across the team. This is accomplished, in part, with code review. Code review helps developers learn the code base, as well as help them learn new technologies and techniques that grow their skill sets.
Agile teams can realize huge benefits because work is decentralized across the team. No one is the only person who knows a specific part of the code base. Simply put, code reviews help facilitate knowledge sharing across the code base and across the team. Full stack engineers can tackle front-end work as well as server-side work.
Code review is not just a senior team member reviewing a junior team member’s code. Code review should happen across the team in every direction. Newer members, with fresh eyes, discover gnarly, time-plauged areas of the code base that need a new perspective. So, code review also helps ensure new insight is tempered with existing knowledge.
Don’t wait for a code review if feedback is needed earlier in the development cycle.
- Developers should review no more than 200 to 400 lines of code (LOC) at a time (over 60 to 90 minutes). The brain can only effectively process so much information at a time; beyond 400 LOC, the ability to find defects diminishes.
- Take your time. Inspection rates should under 500 LOC per hour.
- Do not review for more than 60 minutes at a time. When people engage in any activity requiring concentrated effort over a period of time, performance starts dropping off after about 60 minutes. Studies show that taking breaks from a task over a period of time can greatly improve quality of work.
- Set goals and capture metrics, to measure and improve the review efficiency. A metrics-driven code review tool gathers data automatically so that your information is accurate and without human bias.
- Authors should annotate source code before the review. As an added benefit, the author will often find additional errors before the peer review even begins. wcfNote: maybe here we can have a documentation review.
- Write checklists and build upon them. It's very likely that each person on your team makes the same 10 mistakes over and over.
- Establish a process for fixing defects found. The best way to ensure that defects are fixed is to use a collaborative code review tool that allows reviewers to log bugs, discuss them with the author, and approve changes in the code. Without an automated tool, bugs found in review likely aren´t logged in the team´s usual defect tracking system because they are found before code is released to QA.
- Foster a positive code review culture. Peer review can put strain on interpersonal team relationships. It´s difficult to have every piece of work critiqued by peers and to have management evaluating and measuring defect density in your code. Therefore, in order for peer code review to be successful, it´s extremely important that mangers create a culture of collaboration and learning in peer review. Defects found in peer review are not an acceptable rubric by which to evaluate team members. Reports pulled from peer code reviews should never be used in performance reports. If personal metrics become a basis for compensation or promotion, developers will become hostile toward the process and naturally focus on improving personal metrics rather than writing better overall code.
- Embrace the subconscious implications of peer review. Review everyday. The knowledge that others will be examining their work naturally drives people to produce a better product. This "Ego Effect" naturally incentivizes developers to write cleaner code because their peers will certainly see it.
- Practice lightweight code reviews. Lightweight code review takes less than 20% the time of formal reviews and finds just as many bugs!
Initiate a code review after all the code has been written and automated tests have been run and passed–but before the code is merged upstream. This ensures the code reviewer’s time is spent checking for things machines miss, and prevents poor coding decisions from polluting the main line of development.
- “This code seems too complicated”. Complexity is often mistaken for cleverness and effort. It feels good to write complex code but unfortunately it usually feels bad to read it, and in complexity hide bugs. Simpler is always better.
- “I know a better way...”. In that phrase there is sharing, perhaps debate, ultimately education. This is especially powerful when the “better way” reduces line-count.
- Sanity-check Decisions. Did the developer really mean to delete that file? Did they know there’s already an interface that contains that function? Is a 400-line class file really a good idea? A good code review also asks the high-level questions.
- Does the new code conform to existing style guidelines?
- Looking at the requirements, are all cases fully implemented?
- Are the documentation up to date?
- Are the new automated tests sufficient for the new code? Do existing automated tests need to be rewritten to account for changes in the code?
- Are there any obvious logic errors in the code? Unbounce expert says finding bugs is not the primary goal but a side product.