Slide 1

Slide 1 text

Code Reviewing Like A Champion Jonathan Maltz - @maltzj

Slide 2

Slide 2 text

No content

Slide 3

Slide 3 text

No content

Slide 4

Slide 4 text

No content

Slide 5

Slide 5 text

@maltzj What Will We Talk About? ● Why Do Code Review ● Basic Code Review Outline ● How to Give Code Review Feedback

Slide 6

Slide 6 text

@maltzj Why do Code Review?

Slide 7

Slide 7 text

@maltzj Make Sure Code is Correct

Slide 8

Slide 8 text

@maltzj Make Sure Code is Well-Designed

Slide 9

Slide 9 text

@maltzj Share Knowledge

Slide 10

Slide 10 text

@maltzj Basic Code Review Outline

Slide 11

Slide 11 text

@maltzj 1. Build Context

Slide 12

Slide 12 text

@maltzj What’s Success? ● You understand the goal of the change ● You understand what is out of scope ● You understand any high-level decisions

Slide 13

Slide 13 text

@maltzj What’s Failure? ● Jumping into a code review right away ● Not calling for backup as necessary ○ Especially important at boundaries

Slide 14

Slide 14 text

@maltzj Authors are just as important here

Slide 15

Slide 15 text

@maltzj

Slide 16

Slide 16 text

@maltzj Protip: pre-filled templates

Slide 17

Slide 17 text

@maltzj 2. Evaluate Closer

Slide 18

Slide 18 text

@maltzj What’s Success? ● High-level design feedback ● Bug-finding + edge case coverage ● Test coverage ● Naming ● Sharing new patterns

Slide 19

Slide 19 text

@maltzj What’s Failure? ● Commenting on anything a tool can catch ○ PMD ○ Checkstyle ○ Findbugs ○ Pre-commit hooks ● Discussing the same things repeatedly

Slide 20

Slide 20 text

@maltzj 3. Take a Step Back

Slide 21

Slide 21 text

@maltzj Could this be done easier/better/faster?

Slide 22

Slide 22 text

@maltzj How to Give Feedback

Slide 23

Slide 23 text

@maltzj ● Discussion and collaboration ● Authors owning their changes throughout the process ● People being excited about receiving feedback What Do You Want To Encourage?

Slide 24

Slide 24 text

@maltzj

Slide 25

Slide 25 text

@maltzj Feedback that makes people feel worse

Slide 26

Slide 26 text

@maltzj ● “Your style here is inconsistent. Align your braces with our code style.” ● “This has a bug when running on Lollipop which will cause a crash. Fix it.” ● “Why did you do this refactor? The code was better in its previous form.” How Can That Happen?

Slide 27

Slide 27 text

@maltzj

Slide 28

Slide 28 text

@maltzj “This code is good. Let’s make it even better.”

Slide 29

Slide 29 text

@maltzj 1. Start With Facts

Slide 30

Slide 30 text

@maltzj “The style guidelines say you should put braces on the same line and this code puts braces on a new line. Align your braces with the styleguide.”

Slide 31

Slide 31 text

@maltzj

Slide 32

Slide 32 text

@maltzj Opinion == Facts

Slide 33

Slide 33 text

@maltzj Static methods cannot be mocked in normal JUnit tests.

Slide 34

Slide 34 text

@maltzj Static methods cannot be mocked in normal JUnit tests.

Slide 35

Slide 35 text

@maltzj Static methods are bad. They cannot be mocked in normal JUnit tests.

Slide 36

Slide 36 text

@maltzj Link to External Resources

Slide 37

Slide 37 text

@maltzj 2. Invite a Discussion

Slide 38

Slide 38 text

@maltzj “The style guidelines say you should put braces on the same line and this code puts braces on a new line. Can you align your braces with the styleguide?”

Slide 39

Slide 39 text

@maltzj 3. Replace “You” with “We” or “Us”

Slide 40

Slide 40 text

@maltzj “The style guidelines say we should put braces on the same line and this code puts braces on a new line. Can we align the braces with our styleguide?”

Slide 41

Slide 41 text

@maltzj

Slide 42

Slide 42 text

@maltzj

Slide 43

Slide 43 text

@maltzj Rule of Three

Slide 44

Slide 44 text

@maltzj “On a scale of 1-10 I care about this at about a 5. What about you? What are your major concerns?”

Slide 45

Slide 45 text

@maltzj ● Different concerns? Find a third way ● Large Difference? Principle of the person who cares the most in the code review ○ Protip: This shouldn’t always be you ● Both high? Maybe need a larger decision What to do with this?

Slide 46

Slide 46 text

@maltzj Celebrate the Good Stuff

Slide 47

Slide 47 text

@maltzj

Slide 48

Slide 48 text

@maltzj External Links ● Yelp’s Code Review Guidelines ● Writing Commit Messages ● How to Use Code Review To Execute Someone’s Soul ● Creating a Strong Code Review Culture ● Honesty Kindness, Inspiration: Pick Three ● Bettercode.reviews ● Giving and Getting Technical Help ● Crucial Conversations ● Pre-commit