Slide 1

Slide 1 text

Code Review Best Practice Trisha Gee (@trisha_gee)
 Developer Advocate & Java Champion, JetBrains

Slide 2

Slide 2 text

No content

Slide 3

Slide 3 text

This code works

Slide 4

Slide 4 text

Having Opinions On Code Is An Occupational Hazard

Slide 5

Slide 5 text

Having Opinions On Code Is An Occupational Requirement

Slide 6

Slide 6 text

Are we harder on other people’s code than our own?

Slide 7

Slide 7 text

What should we be looking for in a Code Review?

Slide 8

Slide 8 text

http://jb.gg/book/codereview

Slide 9

Slide 9 text

• Gateway Reviews • Knowledge Sharing • Early Design Feedback Different workflows https://blog.jetbrains.com/upsource/tag/code-review-workflows/

Slide 10

Slide 10 text

What should you look for when reviewing code?

Slide 11

Slide 11 text

It Depends

Slide 12

Slide 12 text

My First Code Review My job is to Find Problems

Slide 13

Slide 13 text

Anti-Pattern Nit picking

Slide 14

Slide 14 text

Anti-Pattern Design changes when the code works

Slide 15

Slide 15 text

Anti-Pattern Inconsistent feedback

Slide 16

Slide 16 text

Anti-Pattern The Ghost Reviewer

Slide 17

Slide 17 text

Anti-Pattern Ping pong reviews

Slide 18

Slide 18 text

Developers hate code reviews

Slide 19

Slide 19 text

Code Reviews are a 
 Massive Waste of Time

Slide 20

Slide 20 text

Take a step back…

Slide 21

Slide 21 text

1. Why?

Slide 22

Slide 22 text

Ensure code meets standards

Slide 23

Slide 23 text

Find bugs

Slide 24

Slide 24 text

Ensure code does what it’s supposed to

Slide 25

Slide 25 text

Check code is understandable

Slide 26

Slide 26 text

Share knowledge

Slide 27

Slide 27 text

Collaborate on design

Slide 28

Slide 28 text

Evolve application code

Slide 29

Slide 29 text

2. When?

Slide 30

Slide 30 text

• During implementation? • When it’s ready to merge? • After it’s been merged? When do you review?

Slide 31

Slide 31 text

• When everyone agrees? • When a gatekeeper agrees? • When all comments are addressed? When is the review complete?

Slide 32

Slide 32 text

3. Who?

Slide 33

Slide 33 text

Who reviews the code?

Slide 34

Slide 34 text

Who signs it off?

Slide 35

Slide 35 text

4. Where?

Slide 36

Slide 36 text

Pairing

Slide 37

Slide 37 text

Showing code to a colleague at a computer

Slide 38

Slide 38 text

Mob reviewing in a conference room

Slide 39

Slide 39 text

Remote screen-sharing

Slide 40

Slide 40 text

In the IDE, checking out a commit or branch

Slide 41

Slide 41 text

Using code review software

Slide 42

Slide 42 text

5. What?

Slide 43

Slide 43 text

1. Why 2. When 3. Who 4. Where Requires you to know:

Slide 44

Slide 44 text

• Fit with the overall architecture • SOLID principles, Domain Driven Design, Design Patterns or other paradigms of choice • New code follows team’s current practices • Code is in the right place • Code reuse • Over-engineering • Readable code and tests • Testing the right things • Exception error messages • Subtle bugs • Security • Regulatory requirements • Performance • Documentation and/or help files been updated • Spelling, punctuation & grammar on user messages What to look for https://blog.jetbrains.com/upsource/tag/what-to-look-for/

Slide 45

Slide 45 text

Human reviewers should be doing what cannot be automated

Slide 46

Slide 46 text

Understand the constraints

Slide 47

Slide 47 text

Why: Knowledge Sharing Purpose isn’t to reject the code

Slide 48

Slide 48 text

Why: Knowledge Sharing Focus is on learning what the code does and why

Slide 49

Slide 49 text

When: At the End Too late for design

Slide 50

Slide 50 text

When: At the End Should have set of checks

Slide 51

Slide 51 text

6. How?

Slide 52

Slide 52 text

Automate Everything You Can

Slide 53

Slide 53 text

Submitting for review Reviews should be small

Slide 54

Slide 54 text

Submitting for review Annotate your code

Slide 55

Slide 55 text

Reviewing Should be clear Who is reviewing

Slide 56

Slide 56 text

Reviewing Respond in a timely fashion

Slide 57

Slide 57 text

Reviewing Checklist of What to look for

Slide 58

Slide 58 text

Comments Bear in mind Why, When and What

Slide 59

Slide 59 text

Comments Be constructive

Slide 60

Slide 60 text

Comments Be specific

Slide 61

Slide 61 text

Accept or Reject

Slide 62

Slide 62 text

Accept or Raise Concern Next steps should be clear

Slide 63

Slide 63 text

Making changes Respond in a timely fashion

Slide 64

Slide 64 text

Making changes Respond to comments

Slide 65

Slide 65 text

Resolving The goal is to accept the review

Slide 66

Slide 66 text

Resolving Should be clear Who signs it off

Slide 67

Slide 67 text

Resolving …and When

Slide 68

Slide 68 text

Code Reviews Suck Less When…

Slide 69

Slide 69 text

1. Why 2. When 3. Who 4. Where 5. What 6. How The process is clear

Slide 70

Slide 70 text

Not to prove how clever you are The Goal Is To Ship The Code

Slide 71

Slide 71 text

http://bit.ly/CRGood