Effective Code Review

Transcript

1. EFFECTIVE
   CODE
   REVIEW
   

   View Slide

2. EFFECTIVE
   CODE
   REVIEW
   

   View Slide

3. Who am I?
   

   View Slide

4. @d0ugal
   

   View Slide

5. Raise your
   hand…
   

   View Slide

6. Not doing code review?
   

   View Slide

7. “the average defect detection rate is only
   25 percent for unit testing, 35 percent for
   function testing, and 45 percent for
   integration testing. In contrast, the
   average effectiveness of design and code
   inspections are 55 and 60 percent”
   Code Complete by Steve McConnell
   

   View Slide

8. “The only hurdle to a code review is
   finding a developer you respect to do it,
   and making the time to perform the
   review. Once you get started, I think you'll
   quickly find that every minute you spend
   in a code review is paid back tenfold.”
   Jeff Atwood (Coding Horror)
   

   View Slide

9. “Formal design and code inspections […]
   often top 85 percent in defect removal
   efficiency and average about 65 percent”
   Measuring Defect Potentials and Defect Removal Efficiency
   

   View Slide

10. Code Review Goals
    Expectation vs Outcome
    

    View Slide

11. “While finding defects remains the main
    motivation for review, reviews are less
    about defects than expected and instead
    provide additional benefits such as
    knowledge transfer, increased team
    awareness, and creation of alternative
    solutions to problems.”
    Expectations, Outcomes, and Challenges Of Modern Code Review
    

    View Slide

12. Comment Outcomes
    1. Code Improvements (29%)
    2. Understanding
    3. Social Communications
    4. Defects (14%)
    5. External Impact
    6. Testing
    7. Review Tool
    8. Knowledge Transfer
    9. Misc
    

    View Slide

13. Authors
    Reviewers
    

    View Slide

14. VS
    Authors
    Reviewers
    

    View Slide

15. Code Review
    Code Discussion
    Code Collaboration
    …
    

    View Slide

16. &
    Authors
    Reviewers
    

    View Slide

17. Authoring Changes
    

    View Slide

18. Don’t start with code!
    https://flic.kr/p/cexrh1
    

    View Slide

19. Adhere to Project Guidelines
    Write test.
    Write documentation.
    Test the relevant platforms.
    Follow the Style guide.
    

    View Slide

20. Provide Context
    https://flic.kr/p/nZpgc6
    

    View Slide

21. Small & Contained
    “code review:
    10 LOC - 9 issues,
    500 LOC - looks fine”
    Mikhail Garber
    (@mikhailgarber)
    

    View Slide

22. “Its regression coefficients are positive,
    indicating that larger patches lead to a
    higher likelihood of reviewers missing
    some bugs. Similarly, number of files has a
    good explanatory power in all four
    systems.”
    Investigating Code Review Quality:
    Do People and Participation Matter?
    

    View Slide

23. Opening a Review is the start
    Start of the conversation
    Don’t ask for it to be merged, ask for
    it to be reviewed
    

    View Slide

24. Relinquish Ownership
    “0% thankfully. Coders act like
    they've painted a masterpiece
    and tend to debate every piece
    of feedback.”
    Mark Litwintschik
    (@marklit82)
    

    View Slide

25. 
    Code Review is hard.
    

    View Slide

26. Reviewing Changes
    

    View Slide

27. Shared Responsibility
    

    View Slide

28. Contributions == Puppies
    

    View Slide

29. Everyone Reviews
    Juniors. Seniors.
    Review to learn, verify and teach. Not
    necessarily in that order.
    

    View Slide

30. Keep reviewers on the same page
    If they are all reviewing to different
    rules, it will never make sense
    

    View Slide

31. Automation
    https://flic.kr/p/5Pnxus
    

    View Slide

32. Remove the Bikeshed
    https://flic.kr/p/8qqMca
    

    View Slide

33. Multiple Reviewers
    

    View Slide

34. Frequent, Short Reviews
    https://flic.kr/p/atDNLR
    

    View Slide

35. Constructive criticism and Praise
    It’s easy to just point out the bad
    things, but when somebody teaches
    you something - “I didn’t know you
    could do that!” moments - let them
    know.
    

    View Slide

36. Be Polite and aware of tone
    Some things can come across overly
    negative.
    “Why didn’t you do …?”
    Sounds more negative written than
    in person. Replace with
    “Could we do this …?”
    

    View Slide

37. Never harsh. Never Personal
    https://flic.kr/p/efcTcb
    

    View Slide

38. 
    Writing Code is hard.
    

    View Slide

39. Collaboration
    Help each other.
    Automate what you can.
    Be kind to yourself.
    

    View Slide

40. Tooling
    GitHub? Gerrit? Phabricator?
    GitLab? Review Board?
    

    View Slide

41. Review Before The
    Merge
    

    View Slide

42. GitHub
    Loose workflow. Labels are
    useful.
    Simple UI.
    

    View Slide

43. Gerrit
    Very defined. Multiple
    reviewers.
    

    View Slide

44. Code Review Data
    

    View Slide

45. Questions?
    twitter.com/d0ugal
    github.com/d0ugal
    dougal@dougalmatthews.com
    (Sort-of related; OpenStack Open
    Space tomorrow afternoon)
    

    View Slide