Upgrade to Pro
— share decks privately, control downloads, hide ads and more …
Speaker Deck
Features
Speaker Deck
PRO
Sign in
Sign up for free
Search
Search
There's More to Code Reviews than You Might Thi...
Search
Daniel Shaw
October 14, 2016
Programming
0
110
There's More to Code Reviews than You Might Think ✩
Given at Re:Develop conference, October 2016.
Daniel Shaw
October 14, 2016
Tweet
Share
More Decks by Daniel Shaw
See All by Daniel Shaw
There's more to code review than you might think
thatdamnqa
1
310
Educating Enfys (lightning talk)
thatdamnqa
0
260
There's more to code review than you might think
thatdamnqa
0
270
There's More To Code Reviews Than You Might Think (lightning talk)
thatdamnqa
0
260
The Less Dull Bits of Testing
thatdamnqa
0
250
Other Decks in Programming
See All in Programming
個人軟體時代
ethanhuang13
0
320
Amazon RDS 向けに提供されている MCP Server と仕組みを調べてみた/jawsug-okayama-2025-aurora-mcp
takahashiikki
1
100
ファインディ株式会社におけるMCP活用とサービス開発
starfish719
0
250
1から理解するWeb Push
dora1998
7
1.8k
@Environment(\.keyPath)那么好我不允许你们不知道! / atEnvironment keyPath is so good and you should know it!
lovee
0
100
Oracle Database Technology Night 92 Database Connection control FAN-AC
oracle4engineer
PRO
1
430
さようなら Date。 ようこそTemporal! 3年間先行利用して得られた知見の共有
8beeeaaat
3
1.4k
2025 年のコーディングエージェントの現在地とエンジニアの仕事の変化について
azukiazusa1
20
11k
はじめてのMaterial3 Expressive
ym223
2
170
オープンセミナー2025@広島「君はどこで動かすか?」アンケート結果
satoshi256kbyte
0
260
CJK and Unicode From a PHP Committer
youkidearitai
PRO
0
110
Updates on MLS on Ruby (and maybe more)
sylph01
1
180
Featured
See All Featured
Stop Working from a Prison Cell
hatefulcrawdad
271
21k
Fashionably flexible responsive web design (full day workshop)
malarkey
407
66k
Making the Leap to Tech Lead
cromwellryan
135
9.5k
Optimizing for Happiness
mojombo
379
70k
Building a Modern Day E-commerce SEO Strategy
aleyda
43
7.6k
How to train your dragon (web standard)
notwaldorf
96
6.2k
[RailsConf 2023] Rails as a piece of cake
palkan
57
5.8k
Let's Do A Bunch of Simple Stuff to Make Websites Faster
chriscoyier
507
140k
Sharpening the Axe: The Primacy of Toolmaking
bcantrill
44
2.5k
The Cult of Friendly URLs
andyhume
79
6.6k
A Modern Web Designer's Workflow
chriscoyier
696
190k
Music & Morning Musume
bryan
46
6.8k
Transcript
THERE’S MORE TO CODE REVIEW THAN YOU MIGHT THINK Clair
Shaw @clairs · clairshaw.co.uk
What are Code reviews for?
Code reviews are not an indication of anybody's abilities
☐ Check code style ☐ Review the configuration ☐ Check
forwards compatibility ☐ Review the documentation ☐ Review the commit message ☐ Double check the code ☐ Double check the tests Code review checklist
Check code style
Check code style Pick a house style, use it, communicate
it
Check code style Pick a house style, use it, communicate
it
Check code style Don't get too distracted by checking code
style
Check code style Automate if you can. Bad news can
be better received by a cruel and lifeless script
Review the configuration
Review the configuration { "require": { "usefultool/tool": "1.1" } }
Review the configuration { "require": { "usefultool/tool": "1.2.1" } }
Review the configuration { "require": { "usefultool/tool": "^1.2" } }
Check forwards compatibility
Check forwards compatibility PHP 5.6.26 PHP 7.0.11
Check forwards compatibility jQuery 1 jQuery 2
Check forwards compatibility jQuery 1 jQuery 2 jQuery 3
None
None
Review the documentation
Review the documentation Ensure changes make sense, and are correct
Review the documentation Make sure code changes are documented
Review the commit message
Review the commit message Update usefultool.
Review the commit message Update UsefulTool lib Wider company policy
dictates that the latest UsefulTool v1 should be used. Update Composer config to reflect this. [MYCOOLAPP-1234]
Double check the code
Double check the code if (in_array($user_id, $permitted_users)) { $this->showRestrictedSecrets(); }
else { $this->accessDenied(); }
Double check the code $permitted = ['2', '4', '5', 'cake'];
$input = 0; if (in_array($input, $permitted)) { echo "I'm in!"; }
Double check the code Is the code testable?
Double check the code if (in_array($user_id, $permitted_users)) { $this->showRestrictedSecrets(); }
else { $this->accessDenied(); }
Double check the code function is_allowed_access($input, $expected) { if (in_array($input,
$expected), true) { return true; } else { return false; } }
Double check the code function is_allowed_access($input, $expected) { if (in_array($input,
$expected), true) { return true; } else { return false; } } function testCheckAccess() { $input = 2; $expected = ['2', '4', '5']; assertTrue( $class->is_allowed_access( $input, $expected ); }
Double check the tests
Double check the tests Test for failures too
Double check the tests function is_allowed_access($input, $expected) { if (in_array($input,
$expected), true) { return true; } else { return false; } } function testCheckAccess() { $input = 2; $expected = ['2', '4', '5']; assertTrue( $class->is_allowed_access( $input, $expected ); }
Double check the tests function testCheckAccess() { $input = 2;
$expected = ['2', '4', '5']; assertTrue( $class->is_allowed_access( $input, $expected ); } function testCheckAccess_isStrict() { $input = 0; $expected = ['2', '4', '5', 'cake']; assertFalse( $class->is_allowed_access( $input, $expected ); }
Final thoughts…
Final thoughts… All output should be reviewed. It’s not personal
Final thoughts… Often, a peer review can teach something to
two people
Introducing Code Reviews
Introducing Code Reviews Step 1: Keep watching
Introducing Code Reviews Step 1: Keep watching Step 2: Ask
questions
Introducing Code Reviews Step 1: Keep watching Step 2: Ask
questions Step 3: Go to step 1
None
☑ Check code style ☑ Review the configuration ☑ Check
forwards compatibility ☑ Review the documentation ☑ Review the commit message ☑ Double check the code ☑ Double check the tests Code review checklist
Thanks.
[email protected]
@clairs