Code Reviewing Like a Champion

Transcript

1. Code Reviewing Like A
   Champion
   Jonathan Maltz - @maltzj
   

   View Slide

2. View Slide

3. View Slide

4. View Slide

5. @maltzj
   What Will We Talk About?
   ● Why Do Code Review
   ● Basic Code Review Outline
   ● How to Give Code Review Feedback
   

   View Slide

6. @maltzj
   Why do Code
   Review?
   

   View Slide

7. @maltzj
   Make Sure Code
   is Correct
   

   View Slide

8. @maltzj
   Make Sure Code
   is Well-Designed
   

   View Slide

9. @maltzj
   Share Knowledge
   

   View Slide

10. @maltzj
    Basic Code
    Review Outline
    

    View Slide

11. @maltzj
    1. Build Context
    

    View Slide

12. @maltzj
    What’s Success?
    ● You understand the goal of the change
    ● You understand what is out of scope
    ● You understand any high-level decisions
    

    View Slide

13. @maltzj
    What’s Failure?
    ● Jumping into a code review right away
    ● Not calling for backup as necessary
    ○ Especially important at boundaries
    

    View Slide

14. @maltzj
    Authors are just
    as important here
    

    View Slide

15. @maltzj
    

    View Slide

16. @maltzj
    Protip: pre-filled
    templates
    

    View Slide

17. @maltzj
    2. Evaluate
    Closer
    

    View Slide

18. @maltzj
    What’s Success?
    ● High-level design feedback
    ● Bug-finding + edge case coverage
    ● Test coverage
    ● Naming
    ● Sharing new patterns
    

    View Slide

19. @maltzj
    What’s Failure?
    ● Commenting on anything a tool can catch
    ○ PMD
    ○ Checkstyle
    ○ Findbugs
    ○ Pre-commit hooks
    ● Discussing the same things repeatedly
    

    View Slide

20. @maltzj
    3. Take a Step
    Back
    

    View Slide

21. @maltzj
    Could this be done
    easier/better/faster?
    

    View Slide

22. @maltzj
    How to Give
    Feedback
    

    View Slide

23. @maltzj
    ● Discussion and collaboration
    ● Authors owning their changes throughout the process
    ● People being excited about receiving feedback
    What Do You Want To Encourage?
    

    View Slide

24. @maltzj
    

    View Slide

25. @maltzj
    Feedback that
    makes people
    feel worse
    

    View Slide

26. @maltzj
    ● “Your style here is inconsistent. Align your braces with
    our code style.”
    ● “This has a bug when running on Lollipop which will
    cause a crash. Fix it.”
    ● “Why did you do this refactor? The code was better in its
    previous form.”
    How Can That Happen?
    

    View Slide

27. @maltzj
    

    View Slide

28. @maltzj
    “This code is
    good. Let’s make
    it even better.”
    

    View Slide

29. @maltzj
    1. Start With
    Facts
    

    View Slide

30. @maltzj
    “The style guidelines say
    you should put braces on
    the same line and this code
    puts braces on a new line.
    Align your braces with the
    styleguide.”
    

    View Slide

31. @maltzj
    

    View Slide

32. @maltzj
    Opinion == Facts
    

    View Slide

33. @maltzj
    Static methods cannot
    be mocked in normal
    JUnit tests.
    

    View Slide

34. @maltzj
    Static methods cannot
    be mocked in normal
    JUnit tests.
    

    View Slide

35. @maltzj
    Static methods are bad.
    They cannot be mocked
    in normal JUnit tests.
    

    View Slide

36. @maltzj
    Link to External
    Resources
    

    View Slide

37. @maltzj
    2. Invite a
    Discussion
    

    View Slide

38. @maltzj
    “The style guidelines say
    you should put braces on
    the same line and this code
    puts braces on a new line.
    Can you align your braces
    with the styleguide?”
    

    View Slide

39. @maltzj
    3. Replace “You”
    with “We” or “Us”
    

    View Slide

40. @maltzj
    “The style guidelines say we
    should put braces on the
    same line and this code puts
    braces on a new line. Can
    we align the braces with our
    styleguide?”
    

    View Slide

41. @maltzj
    

    View Slide

42. @maltzj
    

    View Slide

43. @maltzj
    Rule of Three
    

    View Slide

44. @maltzj
    “On a scale of 1-10 I care
    about this at about a 5.
    What about you? What are
    your major concerns?”
    

    View Slide

45. @maltzj
    ● Different concerns? Find a third way
    ● Large Difference? Principle of the person who cares the
    most in the code review
    ○ Protip: This shouldn’t always be you
    ● Both high? Maybe need a larger decision
    What to do with this?
    

    View Slide

46. @maltzj
    Celebrate the
    Good Stuff
    

    View Slide

47. @maltzj
    

    View Slide

48. @maltzj
    External Links
    ● Yelp’s Code Review Guidelines
    ● Writing Commit Messages
    ● How to Use Code Review To Execute Someone’s Soul
    ● Creating a Strong Code Review Culture
    ● Honesty Kindness, Inspiration: Pick Three
    ● Bettercode.reviews
    ● Giving and Getting Technical Help
    ● Crucial Conversations
    ● Pre-commit
    

    View Slide