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
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