Cultivating a Code Review Culture

Slide 1

Slide 1 text

Cultivating a Code Review Culture Derek Prior thoughtbot

Slide 2

Slide 2 text

No content

Slide 3

Slide 3 text

Why Do We Do Reviews?

Slide 4

Slide 4 text

To Catch Bugs!

Slide 5

Slide 5 text

NO!

Slide 6

Slide 6 text

Okay, well maybe a little bit.

Slide 7

Slide 7 text

Expectations, Outcomes, and Challenges of Modern Code Review

Slide 8

Slide 8 text

Knowledge Transfer

Slide 9

Slide 9 text

Knowledge Transfer Increased Team Awareness

Slide 10

Slide 10 text

Knowledge Transfer Increased Team Awareness Finding Alternative Solutions

Slide 11

Slide 11 text

the discipline of explaining your code to your peers

Slide 12

Slide 12 text

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

Slide 13

Slide 13 text

Strong Code Review Culture

Slide 14

Slide 14 text

Rules of Engagement • As an author...

Slide 15

Slide 15 text

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

Slide 16

Slide 16 text

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

Slide 17

Slide 17 text

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

Slide 18

Slide 18 text

Use type column first in multi-column indexes

Slide 19

Slide 19 text

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

Slide 20

Slide 20 text

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 ﬁrst column in the index is the id column, which is less than ideal. ...

Slide 21

Slide 21 text

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 efﬁcient when there are constraints on the leading (leftmost) columns. ...

Slide 22

Slide 22 text

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

Slide 23

Slide 23 text

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

Slide 24

Slide 24 text

Rules of Engagement • As an author, provide sufﬁcient context. • As a reviewer...

Slide 25

Slide 25 text

Rules of Engagement • As an author, provide sufﬁcient context. • As a reviewer...

Slide 26

Slide 26 text

Ask, don't tell

Slide 27

Slide 27 text

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

Slide 28

Slide 28 text

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

Slide 29

Slide 29 text

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

Slide 30

Slide 30 text

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

Slide 31

Slide 31 text

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

Slide 32

Slide 32 text

Question ALL THE THINGS!

Slide 33

Slide 33 text

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

Slide 34

Slide 34 text

JUST

Slide 35

Slide 35 text

"Why didn't you... ?"

Slide 36

Slide 36 text

Be Positive

Slide 37

Slide 37 text

Socratic Method

Slide 38

Slide 38 text

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

Slide 39

Slide 39 text

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

Slide 40

Slide 40 text

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?

Slide 41

Slide 41 text

Conflict

Slide 42

Slide 42 text

Conflict • We don't agree on the issue

Slide 43

Slide 43 text

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

Slide 44

Slide 44 text

What To Review

Slide 45

Slide 45 text

What To Review • Single Responsibility Principle

Slide 46

Slide 46 text

What To Review • Single Responsibility Principle • Naming

Slide 47

Slide 47 text

What To Review • Single Responsibility Principle • Naming • Complexity

Slide 48

Slide 48 text

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

Slide 49

Slide 49 text

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

Slide 50

Slide 50 text

What About Style?

Slide 51

Slide 51 text

Style • Write it down.

Slide 52

Slide 52 text

Style • Write it down • Outsource it

Slide 53

Slide 53 text

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.

Slide 54

Slide 54 text

Strong Code Review Culture

Slide 55

Slide 55 text

Benefits of a Strong Code Review Culture • Better Code

Slide 56

Slide 56 text

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

Slide 57

Slide 57 text

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

Slide 58

Slide 58 text

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

Slide 59

Slide 59 text

Questions?

Slide 60

Slide 60 text

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

Slide 61

Slide 61 text

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

Slide 62

Slide 62 text

Photo Credits • https://ﬂickr.com/photos/tambako/14539706246 • https://ﬂickr.com/photos/johnjoh/4863006085