Cultivating a Code Review Culture

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.

C537a86fdc5234b3e941a84c154ba034?s=128

Derek Prior

April 22, 2015
Tweet

Transcript

  1. Cultivating a Code Review Culture Derek Prior thoughtbot

  2. None
  3. Why Do We Do Reviews?

  4. To Catch Bugs!

  5. NO!

  6. Okay, well maybe a little bit.

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

  8. Knowledge Transfer

  9. Knowledge Transfer Increased Team Awareness

  10. Knowledge Transfer Increased Team Awareness Finding Alternative Solutions

  11. the discipline of explaining your code to your peers

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

    your peers.
  13. Strong Code Review Culture

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

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

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

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

    Vaynerchuck
  18. Use type column first in multi-column indexes

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

  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. ...
  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. ...
  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. ...
  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.
  24. Rules of Engagement • As an author, provide sufficient context.

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

    • As a reviewer...
  26. Ask, don't tell

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

  28. "What do you think about extracting a service to reduce

    some of this duplication?"
  29. Ask, Don't Tell • What do you think about... ?

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

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

    • Did you consider... ? • Can you clarify... ?
  32. Question ALL THE THINGS!

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

  34. JUST

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

  36. Be Positive

  37. Socratic Method

  38. Rules of Engagement • As an author, provide context. •

    As a reviewer, ask questions rather than making demands.
  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?
  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?
  41. Conflict

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

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

    don't agree on the process
  44. What To Review

  45. What To Review • Single Responsibility Principle

  46. What To Review • Single Responsibility Principle • Naming

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

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

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

    Complexity • Test Coverage • ...
  50. What About Style?

  51. Style • Write it down.

  52. Style • Write it down • Outsource it

  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.
  54. Strong Code Review Culture

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

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

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

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

    • Better Developers • Team Ownership • Healthy Debate
  59. Questions?

  60. Derek Prior • twitter: @derekprior • email: derek@thoughtbot.com • podcast:

    http://bikeshed.fm
  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
  62. Photo Credits • https://flickr.com/photos/tambako/14539706246 • https://flickr.com/photos/johnjoh/4863006085