A presentation by @stuherbert
for @GanbaroDigital
Build A Culture
Of Improvement
Positive
Code Reviews
Slide 2
Slide 2 text
@GanbaroDigital
Huge shoutout to Jo
for asking me
to do this talk.
Slide 3
Slide 3 text
@GanbaroDigital
Do you do code reviews?
Slide 4
Slide 4 text
@GanbaroDigital
Tell me about them.
Slide 5
Slide 5 text
@GanbaroDigital
How do you feel
when your code is reviewed?
Slide 6
Slide 6 text
@GanbaroDigital
How do you feel
when you review
someone else’s code?
Slide 7
Slide 7 text
@GanbaroDigital
Why do you feel
like that?
Slide 8
Slide 8 text
@GanbaroDigital
A positive code review
is one that all participants
actively want to take part in.
Slide 9
Slide 9 text
@GanbaroDigital
To understand code reviews,
it helps to understand the process
that they sit in.
Slide 10
Slide 10 text
@GanbaroDigital
How Tasks Are Typically Done
1. Developer is assigned ticket in JIRA
2. Developer writes code
(and maybe some tests)
3. Best mate reviews code if they have the time
4. Developer merges code
Slide 11
Slide 11 text
@GanbaroDigital
Let’s break that down.
Slide 12
Slide 12 text
@GanbaroDigital
How Tasks Are Typically Done
1. Developer is assigned ticket in JIRA
2. Developer writes code
(and maybe some tests)
3. Best mate reviews code if they have the time
4. Developer merges code
Slide 13
Slide 13 text
@GanbaroDigital
How many organisations
create JIRA tickets
that are ready to code?
Slide 14
Slide 14 text
@GanbaroDigital
How Tasks Are Typically Done
1. Developer is assigned ticket in JIRA
2. Developer writes code
(and maybe some tests)
3. Best mate reviews code if they have the time
4. Developer merges code
Slide 15
Slide 15 text
@GanbaroDigital
How many developers question
what they are told to deliver?
Slide 16
Slide 16 text
@GanbaroDigital
How many developers
have access to the person
they need to question?
Slide 17
Slide 17 text
@GanbaroDigital
How Tasks Are Typically Done
1. Developer is assigned ticket in JIRA
2. Developer writes code
(and maybe some tests)
3. Best mate reviews code if they have the time
4. Developer merges code
Slide 18
Slide 18 text
@GanbaroDigital
How many organisations
require tests alongside code?
Slide 19
Slide 19 text
@GanbaroDigital
How Tasks Are Typically Done
1. Developer is assigned ticket in JIRA
2. Developer writes code
(and maybe some tests)
3. Best mate reviews code if they have the time
4. Developer merges code
Slide 20
Slide 20 text
@GanbaroDigital
How many organisations
define how many tests
must accompany code?
Slide 21
Slide 21 text
@GanbaroDigital
How Tasks Are Typically Done
1. Developer is assigned ticket in JIRA
2. Developer writes code
(and maybe some tests)
3. Best mate reviews code if they have the time
4. Developer merges code
Slide 22
Slide 22 text
@GanbaroDigital
It is natural to seek out people
who are more likely
to agree with us.
Especially in a toxic culture.
Slide 23
Slide 23 text
@GanbaroDigital
How many organisations
require code reviews?
Slide 24
Slide 24 text
@GanbaroDigital
How many organisations
define code review standards?
Slide 25
Slide 25 text
@GanbaroDigital
What standards do you
review against?
Slide 26
Slide 26 text
@GanbaroDigital
PSR-2 is a formatting standard.
It is not a complete
code review standard.
Slide 27
Slide 27 text
@GanbaroDigital
How Tasks Are Typically Done
1. Developer is assigned ticket in JIRA
2. Developer writes code
(and maybe some tests)
3. Best mate reviews code if they have the time
4. Developer merges code
Slide 28
Slide 28 text
@GanbaroDigital
How many organisations
know what is in
their shipped software?
Slide 29
Slide 29 text
@GanbaroDigital
How Tasks Are Typically Done
1. Developer is assigned ticket in JIRA
2. Developer writes code
(and maybe some tests)
3. Best mate reviews code if they have the time
4. Developer merges code
Slide 30
Slide 30 text
@GanbaroDigital
Positive code reviews?!?
Despite the culture,
not because of it.
Slide 31
Slide 31 text
@GanbaroDigital
Unpleasant code reviews
are a side-effect
of organisation culture.
Slide 32
Slide 32 text
@GanbaroDigital
Unpleasant code reviews
are a side-effect of a lack of
deliberate organisation culture.
Slide 33
Slide 33 text
@GanbaroDigital
To fix your code reviews,
you have to fix your culture.
Slide 34
Slide 34 text
@GanbaroDigital
Some cultures are
too toxic to be fixed :-(
Slide 35
Slide 35 text
@GanbaroDigital
No process can overcome
toxic people :-(
Slide 36
Slide 36 text
@GanbaroDigital
Culture is what happens when
management leaves the room.
Slide 37
Slide 37 text
@GanbaroDigital
Culture is a reaction
to management behaviour.
Slide 38
Slide 38 text
@GanbaroDigital
Leaders get the culture
that they live themselves.
Slide 39
Slide 39 text
@GanbaroDigital
Be a better leader
to have a better culture.
Slide 40
Slide 40 text
@GanbaroDigital
How does this help us
do positive code reviews?
Slide 41
Slide 41 text
@GanbaroDigital
Introducing:
the rule of no surprises.
Slide 42
Slide 42 text
@GanbaroDigital
Do not violate
the rule of no surprises.
Slide 43
Slide 43 text
@GanbaroDigital
Set expectations up front.
Slide 44
Slide 44 text
@GanbaroDigital
Set measurable expectations
up front.
Slide 45
Slide 45 text
@GanbaroDigital
Provide evidence
for and against defined
expectations and standards.
Slide 46
Slide 46 text
@GanbaroDigital
Provide evidence
for and against agreed
expectations and standards.
Slide 47
Slide 47 text
@GanbaroDigital
Provide evidence
for and against agreed
expectations and standards.
Slide 48
Slide 48 text
@GanbaroDigital
Leave opinion at the door.
Slide 49
Slide 49 text
@GanbaroDigital
Introducing:
everything must
stand up to scrutiny.
Slide 50
Slide 50 text
@GanbaroDigital
Everything ==
1. Requirements
2. Design
3. Written code
4. Test plan
Slide 51
Slide 51 text
@GanbaroDigital
Everything ==
1. Requirements
2. Design
3. Written code
4. Test plan
Slide 52
Slide 52 text
@GanbaroDigital
People do a better job
when they believe in
what they’re asked to do.
Slide 53
Slide 53 text
@GanbaroDigital
Everything ==
1. Requirements
2. Design
3. Written code
4. Test plan
Slide 54
Slide 54 text
@GanbaroDigital
Teams write better code
when they’re contributing
to a consistent design.
Slide 55
Slide 55 text
@GanbaroDigital
Everything ==
1. Requirements
2. Design
3. Written code
4. Test plan
Slide 56
Slide 56 text
@GanbaroDigital
Everything ==
1. Requirements
2. Design
3. Written code
4. Test plan
Slide 57
Slide 57 text
@GanbaroDigital
Review tests that
cover the happy path.
Slide 58
Slide 58 text
@GanbaroDigital
Review tests that
prove robustness.
Slide 59
Slide 59 text
@GanbaroDigital
Everything ==
1. Requirements
2. Design
3. Written code
4. Test plan
Slide 60
Slide 60 text
@GanbaroDigital
“Do as I say, not as I do.”
How many of you have worked
for someone like that?
Slide 61
Slide 61 text
@GanbaroDigital
Lead by example.
Make sure all of your work
is reviewed.
Slide 62
Slide 62 text
@GanbaroDigital
Lead by example.
Make sure all of your work
meets the review standards.
Slide 63
Slide 63 text
@GanbaroDigital
Only then
do you have the moral right
to review the work
of your colleagues.
Slide 64
Slide 64 text
@GanbaroDigital
Introducing:
everyone is entitled
to a bad day.
Slide 65
Slide 65 text
@GanbaroDigital
It isn’t credible
to expect everyone
to be brilliant every day.
@GanbaroDigital
How We Learn
1. Instruction
2. Coaching
3. Collaboration
Slide 69
Slide 69 text
@GanbaroDigital
How We Learn
1. Instruction
2. Coaching
3. Collaboration
Slide 70
Slide 70 text
@GanbaroDigital
How We Learn
1. Instruction
2. Coaching
3. Collaboration
Slide 71
Slide 71 text
@GanbaroDigital
Not everyone
is comfortable
giving or receiving
instruction.
Slide 72
Slide 72 text
@GanbaroDigital
Not everyone
reaches
the coaching stage.
Slide 73
Slide 73 text
@GanbaroDigital
Guiding Principles
1. The rule of no surprises.
2. Everything must stand up to scrutiny.
3. Everyone is entitled to a bad day.
4. Ask, don’t tell (sometimes).
Slide 74
Slide 74 text
@GanbaroDigital
Discussion:
How do we incorporate
these principles
into a positive review process?
Slide 75
Slide 75 text
@GanbaroDigital
Change the conversation.
Slide 76
Slide 76 text
@GanbaroDigital
1. Make sure only the right people attend.
2. Make sure expectations are known in advance.
3. Discover and correct problems early.
Slide 77
Slide 77 text
@GanbaroDigital
Code Review Attendees
1. Person doing the work.
2. Their boss - team lead or senior dev.
3. Observers who need to learn.
Slide 78
Slide 78 text
@GanbaroDigital
Code Review Attendees
1. Person doing the work.
2. Their boss - team lead or senior dev.
3. Observers who need to learn.
Slide 79
Slide 79 text
@GanbaroDigital
Code Review Attendees
1. Person doing the work.
2. Their boss - team lead or senior dev.
3. Observers who need to learn.
Slide 80
Slide 80 text
@GanbaroDigital
Review Thrice, Ship Once
1. Review task and plan of attack.
2. Review code.
3. Review & approve code + tests.
Slide 81
Slide 81 text
@GanbaroDigital
Review Thrice, Ship Once
1. Review task and plan of attack.
2. Review code.
3. Review & approve code + tests.
Slide 82
Slide 82 text
@GanbaroDigital
Before Writing Code
• Check understanding of the problem.
• Review proposed solution.
• Review proposed test plan.
• Give or withhold approval.
Slide 83
Slide 83 text
@GanbaroDigital
Review Thrice, Ship Once
1. Review task and plan of attack.
2. Review code.
3. Review & approve code + tests.
Slide 84
Slide 84 text
@GanbaroDigital
After Writing Code
• Review developed solution.
• Review revised test plan.
• Give or withhold approval.
Slide 85
Slide 85 text
@GanbaroDigital
Review Thrice, Ship Once
1. Review task and plan of attack.
2. Review code.
3. Review & approve code + tests.
Slide 86
Slide 86 text
@GanbaroDigital
Before Merging
• Repeat test plan execution.
• Review test plan execution results.
• Give or withhold approval.
Slide 87
Slide 87 text
@GanbaroDigital
Break down review points into
general and specific items.
Slide 88
Slide 88 text
@GanbaroDigital
Make everyone
involved in a code review
sign off on quality.
Slide 89
Slide 89 text
@GanbaroDigital
Make everyone
involved in a code review
accountable for everything
that they have approved.
Slide 90
Slide 90 text
@GanbaroDigital
Give everyone
involved in a code review
the right to refuse to sign off
the work.
Slide 91
Slide 91 text
@GanbaroDigital
… as long as their refusal
is evidence-based
against documented
standards & requirements!
Slide 92
Slide 92 text
@GanbaroDigital
Review Thrice, Ship Once
1. Review task and plan of attack.
2. Review code.
3. Review & approve code + tests.
Slide 93
Slide 93 text
@GanbaroDigital
In summary …
Slide 94
Slide 94 text
@GanbaroDigital
The root causes of
unpleasant code reviews
are to be found
a long way upstream.
Slide 95
Slide 95 text
@GanbaroDigital
Agree review standards
up front.
Slide 96
Slide 96 text
@GanbaroDigital
Evidence based,
not opinion based.
Slide 97
Slide 97 text
@GanbaroDigital
Lead by example,
and the right people
will respond in kind.
Slide 98
Slide 98 text
@GanbaroDigital
Review thrice, merge once.
Slide 99
Slide 99 text
@GanbaroDigital
Make everyone
involved in a code review
sign off on quality.
Slide 100
Slide 100 text
@GanbaroDigital
No process can solve
the problem of toxic people …
… except perhaps
the firing process!
Slide 101
Slide 101 text
Would you like to
know more?
We can help.
Stuart Herbert ~ @stuherbert
Founder @GanbaroDigital