Effective Code Review - Speaker Deck

Slide 1

Slide 1 text

EFFECTIVE CODE REVIEW

Slide 2

Slide 2 text

EFFECTIVE CODE REVIEW

Slide 3

Slide 3 text

Who am I?

Slide 4

Slide 4 text

@d0ugal

Slide 5

Slide 5 text

Raise your hand…

Slide 6

Slide 6 text

Not doing code review?

Slide 7

Slide 7 text

“the average defect detection rate is only 25 percent for unit testing, 35 percent for function testing, and 45 percent for integration testing. In contrast, the average eﬀectiveness of design and code inspections are 55 and 60 percent” Code Complete by Steve McConnell

Slide 8

Slide 8 text

“The only hurdle to a code review is finding a developer you respect to do it, and making the time to perform the review. Once you get started, I think you'll quickly find that every minute you spend in a code review is paid back tenfold.” Jeff Atwood (Coding Horror)

Slide 9

Slide 9 text

“Formal design and code inspections […] often top 85 percent in defect removal eﬃciency and average about 65 percent” Measuring Defect Potentials and Defect Removal Eﬃciency

Slide 10

Slide 10 text

Code Review Goals Expectation vs Outcome

Slide 11

Slide 11 text

“While ﬁnding defects remains the main motivation for review, reviews are less about defects than expected and instead provide additional beneﬁts such as knowledge transfer, increased team awareness, and creation of alternative solutions to problems.” Expectations, Outcomes, and Challenges Of Modern Code Review

Slide 12

Slide 12 text

Comment Outcomes 1. Code Improvements (29%) 2. Understanding 3. Social Communications 4. Defects (14%) 5. External Impact 6. Testing 7. Review Tool 8. Knowledge Transfer 9. Misc

Slide 13

Slide 13 text

Authors Reviewers

Slide 14

Slide 14 text

VS Authors Reviewers

Slide 15

Slide 15 text

Code Review Code Discussion Code Collaboration …

Slide 16

Slide 16 text

& Authors Reviewers

Slide 17

Slide 17 text

Authoring Changes

Slide 18

Slide 18 text

Don’t start with code! https://ﬂic.kr/p/cexrh1

Slide 19

Slide 19 text

Adhere to Project Guidelines Write test. Write documentation. Test the relevant platforms. Follow the Style guide.

Slide 20

Slide 20 text

Provide Context https://ﬂic.kr/p/nZpgc6

Slide 21

Slide 21 text

Small & Contained “code review: 10 LOC - 9 issues, 500 LOC - looks ﬁne” Mikhail Garber (@mikhailgarber)

Slide 22

Slide 22 text

“Its regression coeﬃcients are positive, indicating that larger patches lead to a higher likelihood of reviewers missing some bugs. Similarly, number of ﬁles has a good explanatory power in all four systems.” Investigating Code Review Quality: Do People and Participation Matter?

Slide 23

Slide 23 text

Opening a Review is the start Start of the conversation Don’t ask for it to be merged, ask for it to be reviewed

Slide 24

Slide 24 text

Relinquish Ownership “0% thankfully. Coders act like they've painted a masterpiece and tend to debate every piece of feedback.” Mark Litwintschik (@marklit82)

Slide 25

Slide 25 text

Code Review is hard.

Slide 26

Slide 26 text

Reviewing Changes

Slide 27

Slide 27 text

Shared Responsibility

Slide 28

Slide 28 text

Contributions == Puppies

Slide 29

Slide 29 text

Everyone Reviews Juniors. Seniors. Review to learn, verify and teach. Not necessarily in that order.

Slide 30

Slide 30 text

Keep reviewers on the same page If they are all reviewing to diﬀerent rules, it will never make sense

Slide 31

Slide 31 text

Automation https://ﬂic.kr/p/5Pnxus

Slide 32

Slide 32 text

Remove the Bikeshed https://ﬂic.kr/p/8qqMca

Slide 33

Slide 33 text

Multiple Reviewers

Slide 34

Slide 34 text

Frequent, Short Reviews https://ﬂic.kr/p/atDNLR

Slide 35

Slide 35 text

Constructive criticism and Praise It’s easy to just point out the bad things, but when somebody teaches you something - “I didn’t know you could do that!” moments - let them know.

Slide 36

Slide 36 text

Be Polite and aware of tone Some things can come across overly negative. “Why didn’t you do …?” Sounds more negative written than in person. Replace with “Could we do this …?”

Slide 37

Slide 37 text

Never harsh. Never Personal https://ﬂic.kr/p/efcTcb

Slide 38

Slide 38 text

Writing Code is hard.

Slide 39

Slide 39 text

Collaboration Help each other. Automate what you can. Be kind to yourself.

Slide 40

Slide 40 text

Tooling GitHub? Gerrit? Phabricator? GitLab? Review Board?

Slide 41

Slide 41 text

Review Before The Merge

Slide 42

Slide 42 text

GitHub Loose workﬂow. Labels are useful. Simple UI.

Slide 43

Slide 43 text

Gerrit Very deﬁned. Multiple reviewers.

Slide 44

Slide 44 text

Code Review Data

Slide 45

Slide 45 text

Questions? twitter.com/d0ugal github.com/d0ugal [email protected] (Sort-of related; OpenStack Open Space tomorrow afternoon)