Cultivating a Code Review Culture

Code reviews are not about catching bugs. Modern code reviews are about socialization, learning, and teaching.

How can you get the most out of a peer's code review and how can you review code without being seen as overly critical? Reviewing code and writing easily-reviewed features are skills that will make you a better developer and a better teammate.

You will leave this talk with the tools to implement a successful code-review culture. You'll learn how to get more from the reviews you're already getting and how to have more impact with the reviews you leave.

Transcript

1. Cultivating a Code
   Review Culture
   Derek Prior
   thoughtbot
   

   View Slide

2. View Slide

3. Why Do We Do
   Reviews?
   

   View Slide

4. To Catch Bugs!
   

   View Slide

5. NO!
   

   View Slide

6. Okay, well maybe a little bit.
   

   View Slide

7. Expectations,
   Outcomes, and
   Challenges of Modern
   Code Review
   

   View Slide

8. Knowledge Transfer
   

   View Slide

9. Knowledge Transfer
   Increased Team Awareness
   

   View Slide

10. Knowledge Transfer
    Increased Team Awareness
    Finding Alternative Solutions
    

    View Slide

11. the discipline of
    explaining your code to
    your peers
    

    View Slide

12. Code review is the
    discipline of discussing
    your code with your peers.
    

    View Slide

13. Strong Code Review Culture
    

    View Slide

14. Rules of Engagement
    • As an author...
    

    View Slide

15. Rules of Engagement
    • As an author...
    • As a reviewer...
    

    View Slide

16. Rules of Engagement
    • As an author...
    • As a reviewer...
    

    View Slide

17. If content is king, then
    context is God
    — Gary Vaynerchuck
    

    View Slide

18. Use type column first in multi-column indexes
    

    View Slide

19. Use type column first in multi-column indexes
    Fixes #1337
    

    View Slide

20. Use type column first in multi-column indexes
    add_reference can very helpfully add a multi-column index
    when you use it to add a polymorphic reference. However, the
    first column in the index is the id column, which is less than
    ideal.
    ...
    

    View Slide

21. Use type column first in multi-column indexes
    The PostgreSQL docs say:
    A multicolumn B-tree index can be used with query conditions
    that involve any subset of the index's columns, but the index is
    most efficient when there are constraints on the leading (leftmost)
    columns.
    ...
    

    View Slide

22. Use type column first in multi-column indexes
    The MySQL docs say:
    MySQL can use multiple-column indexes for queries that test all
    the columns in the index, or queries that test just the first column,
    the first two columns, the first three columns, and so on. If you
    specify the columns in the right order in the index definition, a
    single composite index can speed up several kinds of queries on
    the same table.
    ...
    

    View Slide

23. Use type column first in multi-column indexes
    In a polymorphic relationship, the type column is much more
    likely to be useful as the first column in an index than the id
    column. That is, I'm more likely to query on type without an id --
    to find all records of a certain type -- than I am to query on id
    without a type.
    

    View Slide

24. Rules of Engagement
    • As an author, provide sufficient context.
    • As a reviewer...
    

    View Slide

25. Rules of Engagement
    • As an author, provide sufficient context.
    • As a reviewer...
    

    View Slide

26. Ask, don't tell
    

    View Slide

27. "Extract a service to
    reduce some of this
    duplication."
    

    View Slide

28. "What do you think
    about extracting a service
    to reduce some of this
    duplication?"
    

    View Slide

29. Ask, Don't Tell
    • What do you think about... ?
    

    View Slide

30. Ask, Don't Tell
    • What do you think about... ?
    • Did you consider... ?
    

    View Slide

31. Ask, Don't Tell
    • What do you think about... ?
    • Did you consider... ?
    • Can you clarify... ?
    

    View Slide

32. Question
    ALL THE THINGS!
    

    View Slide

33. "Why didn't you just... ?"
    

    View Slide

34. JUST
    

    View Slide

35. "Why didn't you... ?"
    

    View Slide

36. Be Positive
    

    View Slide

37. Socratic
    Method
    

    View Slide

38. Rules of Engagement
    • As an author, provide context.
    • As a reviewer, ask questions rather than making demands.
    

    View Slide

39. Rules of Engagement
    • As an author, provide context.
    • As a reviewer, ask questions rather than making demands.
    In Practice
    • How do we handle disagreements?
    

    View Slide

40. Rules of Engagement
    • As an author, provide context.
    • As a reviewer, ask questions rather than making demands.
    In Practice
    • How do we handle disagreements?
    • What should I be reviewing?
    

    View Slide

41. Conflict
    

    View Slide

42. Conflict
    • We don't agree on the issue
    

    View Slide

43. Conflict
    • We don't agree on the issue
    • We don't agree on the process
    

    View Slide

44. What To Review
    

    View Slide

45. What To Review
    • Single Responsibility Principle
    

    View Slide

46. What To Review
    • Single Responsibility Principle
    • Naming
    

    View Slide

47. What To Review
    • Single Responsibility Principle
    • Naming
    • Complexity
    

    View Slide

48. What To Review
    • Single Responsibility Principle
    • Naming
    • Complexity
    • Test Coverage
    

    View Slide

49. What To Review
    • Single Responsibility Principle
    • Naming
    • Complexity
    • Test Coverage
    • ...
    

    View Slide

50. What About Style?
    

    View Slide

51. Style
    • Write it down.
    

    View Slide

52. Style
    • Write it down
    • Outsource it
    

    View Slide

53. Rules of Engagement
    • As an author, provide context.
    • As a reviewer, ask questions rather than making demands.
    In Practice
    • Insist on high quality reviews, but agree to disagree.
    • Review what's important to you.
    

    View Slide

54. Strong Code Review
    Culture
    

    View Slide

55. Benefits of a Strong Code Review Culture
    • Better Code
    

    View Slide

56. Benefits of a Strong Code Review Culture
    • Better Code
    • Better Developers
    

    View Slide

57. Benefits of a Strong Code Review Culture
    • Better Code
    • Better Developers
    • Team Ownership
    

    View Slide

58. Benefits of a Strong Code Review Culture
    • Better Code
    • Better Developers
    • Team Ownership
    • Healthy Debate
    

    View Slide

59. Questions?
    

    View Slide

60. Derek Prior
    • twitter: @derekprior
    • email: derek@thoughtbot.com
    • podcast: http://bikeshed.fm
    

    View Slide

61. Photo Credits
    • https://flickr.com/photos/evilpeacock/6365513881
    • https://flickr.com/photos/urtica/15363980935
    • https://flickr.com/photos/metamerist/36353575
    • https://flickr.com/photos/omcoc/6751047205
    • https://flickr.com/photos/oberazzi/318947873
    • https://flickr.com/photos/vyassaurabh411/7368419622
    • https://flickr.com/photos/wallyg/1659490498
    

    View Slide

62. Photo Credits
    • https://flickr.com/photos/tambako/14539706246
    • https://flickr.com/photos/johnjoh/4863006085
    

    View Slide