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
250
There's more to code review than you might think
thatdamnqa
0
260
There's More To Code Reviews Than You Might Think (lightning talk)
thatdamnqa
0
260
The Less Dull Bits of Testing
thatdamnqa
0
240
Other Decks in Programming
See All in Programming
『自分のデータだけ見せたい!』を叶える──Laravel × Casbin で複雑権限をスッキリ解きほぐす 25 分
akitotsukahara
2
640
AI時代のソフトウェア開発を考える(2025/07版) / Agentic Software Engineering Findy 2025-07 Edition
twada
PRO
91
30k
生成AI時代のコンポーネントライブラリの作り方
touyou
1
230
ソフトウェア品質を数字で捉える技術。事業成長を支えるシステム品質の マネジメント
takuya542
2
13k
地方に住むエンジニアの残酷な現実とキャリア論
ichimichi
5
1.6k
Goで作る、開発・CI環境
sin392
0
240
NPOでのDevinの活用
codeforeveryone
0
850
設計やレビューに悩んでいるPHPerに贈る、クリーンなオブジェクト設計の指針たち
panda_program
6
2.2k
Discover Metal 4
rei315
2
140
Claude Code + Container Use と Cursor で作る ローカル並列開発環境のススメ / ccc local dev
kaelaela
10
5.7k
Webの外へ飛び出せ NativePHPが切り拓くPHPの未来
takuyakatsusa
2
560
High-Level Programming Languages in AI Era -Human Thought and Mind-
hayat01sh1da
PRO
0
780
Featured
See All Featured
The Art of Delivering Value - GDevCon NA Keynote
reverentgeek
15
1.5k
Practical Tips for Bootstrapping Information Extraction Pipelines
honnibal
PRO
20
1.3k
YesSQL, Process and Tooling at Scale
rocio
173
14k
Navigating Team Friction
lara
187
15k
Reflections from 52 weeks, 52 projects
jeffersonlam
351
20k
The MySQL Ecosystem @ GitHub 2015
samlambert
251
13k
A Modern Web Designer's Workflow
chriscoyier
695
190k
What's in a price? How to price your products and services
michaelherold
246
12k
I Don’t Have Time: Getting Over the Fear to Launch Your Podcast
jcasabona
32
2.4k
Chrome DevTools: State of the Union 2024 - Debugging React & Beyond
addyosmani
7
740
Building Applications with DynamoDB
mza
95
6.5k
Making the Leap to Tech Lead
cromwellryan
134
9.4k
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