EFFECTIVE CODE REVIEW EFFECTIVE CODE REVIEW Who am I? @d0ugal - - PowerPoint PPT Presentation

effective code review effective code review who am i
SMART_READER_LITE
LIVE PREVIEW

EFFECTIVE CODE REVIEW EFFECTIVE CODE REVIEW Who am I? @d0ugal - - PowerPoint PPT Presentation

EFFECTIVE CODE REVIEW EFFECTIVE CODE REVIEW Who am I? @d0ugal Raise your hand Not doing code review? the average defect detection rate is only 25 percent for unit testing, 35 percent for function testing, and 45 percent for


slide-1
SLIDE 1

EFFECTIVE CODE REVIEW

slide-2
SLIDE 2

EFFECTIVE CODE REVIEW

slide-3
SLIDE 3

Who am I?

slide-4
SLIDE 4

@d0ugal

slide-5
SLIDE 5

Raise your hand…

slide-6
SLIDE 6

Not doing code review?

slide-7
SLIDE 7

“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 effectiveness of design and code inspections are 55 and 60 percent”

Code Complete by Steve McConnell

slide-8
SLIDE 8

“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

“Formal design and code inspections […]

  • ften top 85 percent in defect removal

efficiency and average about 65 percent”

Measuring Defect Potentials and Defect Removal Efficiency

slide-10
SLIDE 10

Code Review Goals

Expectation vs Outcome

slide-11
SLIDE 11

“While finding defects remains the main motivation for review, reviews are less about defects than expected and instead provide additional benefits 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

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

Authors Reviewers

slide-14
SLIDE 14

VS

Authors Reviewers

slide-15
SLIDE 15

Code Review

Code Discussion Code Collaboration …

slide-16
SLIDE 16

&

Authors Reviewers

slide-17
SLIDE 17

Authoring Changes

slide-18
SLIDE 18

Don’t start with code!

https://flic.kr/p/cexrh1

slide-19
SLIDE 19

Adhere to Project Guidelines

Write test. Write documentation. Test the relevant platforms. Follow the Style guide.

slide-20
SLIDE 20

Provide Context

https://flic.kr/p/nZpgc6

slide-21
SLIDE 21

Small & Contained

“code review: 10 LOC - 9 issues, 500 LOC - looks fine”

Mikhail Garber (@mikhailgarber)

slide-22
SLIDE 22

“Its regression coefficients are positive, indicating that larger patches lead to a higher likelihood of reviewers missing some bugs. Similarly, number of files has a good explanatory power in all four systems.”

Investigating Code Review Quality: Do People and Participation Matter?

slide-23
SLIDE 23

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

Relinquish Ownership

“0% thankfully. Coders act like they've painted a masterpiece and tend to debate every piece

  • f feedback.”

Mark Litwintschik (@marklit82)

slide-25
SLIDE 25

</Authoring Changes>

Code Review is hard.

slide-26
SLIDE 26

Reviewing Changes

slide-27
SLIDE 27

Shared Responsibility

slide-28
SLIDE 28

Contributions == Puppies

slide-29
SLIDE 29

Everyone Reviews

  • Juniors. Seniors.

Review to learn, verify and teach. Not necessarily in that order.

slide-30
SLIDE 30

Keep reviewers on the same page

If they are all reviewing to different rules, it will never make sense

slide-31
SLIDE 31

Automation

https://flic.kr/p/5Pnxus

slide-32
SLIDE 32

Remove the Bikeshed

https://flic.kr/p/8qqMca

slide-33
SLIDE 33

Multiple Reviewers

slide-34
SLIDE 34

Frequent, Short Reviews

https://flic.kr/p/atDNLR

slide-35
SLIDE 35

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

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

Never harsh. Never Personal

https://flic.kr/p/efcTcb

slide-38
SLIDE 38

</Reviewing Changes>

Writing Code is hard.

slide-39
SLIDE 39

Collaboration

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

slide-40
SLIDE 40

Tooling

GitHub? Gerrit? Phabricator? GitLab? Review Board?

slide-41
SLIDE 41

Review Before The Merge

slide-42
SLIDE 42

GitHub

Loose workflow. Labels are useful. Simple UI.

slide-43
SLIDE 43

Gerrit

Very defined. Multiple reviewers.

slide-44
SLIDE 44

Code Review Data

slide-45
SLIDE 45

Questions?

twitter.com/d0ugal github.com/d0ugal dougal@dougalmatthews.com (Sort-of related; OpenStack Open Space tomorrow afternoon)