INTRO TO
CODE REVIEWS
KEVIN LONDON
www.kevinlondon.com
@kevin_london
Slide 2
Slide 2 text
What are code
reviews?
Slide 3
Slide 3 text
A discussion.
Slide 4
Slide 4 text
Why bother?
Slide 5
Slide 5 text
http://bit.ly/ms-code-reviews
Slide 6
Slide 6 text
maybe find bugs
Slide 7
Slide 7 text
Share
knowledge
Slide 8
Slide 8 text
Improve
solutions
Slide 9
Slide 9 text
Grow as a developer
Slide 10
Slide 10 text
“Talk with other programmers;
read other programs.
This is more important than
any book or training course.”
-Peter Norvig, "Teach Yourself Programming in Ten Years"
Slide 11
Slide 11 text
What should I check?
Slide 12
Slide 12 text
Find what matters
for your team
Slide 13
Slide 13 text
Areas I check:
Slide 14
Slide 14 text
Does it
solve the problem?
Slide 15
Slide 15 text
Does it
have good design?
Slide 16
Slide 16 text
Does it
have good tests?
Slide 17
Slide 17 text
Is it
well-documented?
http://bit.ly/teach-dont-tell
Slide 18
Slide 18 text
Is it
secure?
http://bit.ly/owasp-review-top-9
Slide 19
Slide 19 text
Is it
secure?
http://bit.ly/owasp-review-top-9
• Pickle?
Slide 20
Slide 20 text
Is it
secure?
http://bit.ly/owasp-review-top-9
• Pickle?
• DEBUG=True?
Slide 21
Slide 21 text
Is it
secure?
http://bit.ly/owasp-review-top-9
• Pickle?
• DEBUG=True?
• Memory leaks?
Slide 22
Slide 22 text
Is it
secure?
http://bit.ly/owasp-review-top-9
• Pickle?
• DEBUG=True?
• Memory leaks?
• Subprocess shell=True?
Slide 23
Slide 23 text
Style
http://bit.ly/beyond-pep8
Slide 24
Slide 24 text
“I’ve seen quite a few code
reviews where someone
commented on formatting
while missing the fact that
there were security issues
or data model issues.”
Slide 25
Slide 25 text
More suggestions:
http://bit.ly/cr-best-practices
Slide 26
Slide 26 text
How should we
do them?
Slide 27
Slide 27 text
asynchronous
Slide 28
Slide 28 text
asynchronous
and
informal
Slide 29
Slide 29 text
Make them part of
your normal process
Slide 30
Slide 30 text
Keep the
feedback loop
short
Slide 31
Slide 31 text
As a Reviewer:
Slide 32
Slide 32 text
Ask questions
Slide 33
Slide 33 text
Be kind.
Who the hell wrote this?
Oh, I did.
Slide 34
Slide 34 text
You will
not
catch
everything.
Slide 35
Slide 35 text
Aim for
300-400
lines of code / hour
http://bit.ly/smartbear-best-practices
Slide 36
Slide 36 text
Positive criticism
It’s about the code, not the developer.
Slide 37
Slide 37 text
Point out
good things
Slide 38
Slide 38 text
No “why”s
Slide 39
Slide 39 text
No “why”s
No “just”s
Slide 40
Slide 40 text
No “why”s
No “just”s
No shaming
http://bit.ly/giving-technical-help
Slide 41
Slide 41 text
As a Developer:
Slide 42
Slide 42 text
Ask questions
Slide 43
Slide 43 text
Pre-review
your code
git add
git diff —staged
Slide 44
Slide 44 text
Be open to feedback
Slide 45
Slide 45 text
Rotate reviewers
Slide 46
Slide 46 text
Provide context
Slide 47
Slide 47 text
COMMUNICATION
AND
COLLABORATION
YIELD
USEFUL CODE REVIEWS
Slide 48
Slide 48 text
IF YOU’RE NOT DOING
CODE REVIEWS YET:
TRY THEM!
Slides:
http://bit.ly/code-reviews-ladjango