Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Effective Code Review

Effective Code Review

Dougal Matthews

July 25, 2016
Tweet

More Decks by Dougal Matthews

Other Decks in Programming

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
    [email protected]
    (Sort-of related; OpenStack Open
    Space tomorrow afternoon)

    View Slide