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
260
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
Reactの歴史を振り返る
tutinoko
1
180
書き捨てではなく継続開発可能なコードをAIコーディングエージェントで書くために意識していること
shuyakinjo
1
280
Flutterと Vibe Coding で個人開発!
hyshu
1
250
Scale out your Claude Code ~自社専用Agentで10xする開発プロセス~
yukukotani
9
2.2k
대규모 트래픽을 처리하는 프론트 개발자의 전략
maryang
0
120
ワープロって実は計算機で
pepepper
2
1.3k
Google I/O Extended Incheon 2025 ~ What's new in Android development tools
pluu
1
290
自作OSでDOOMを動かしてみた
zakki0925224
1
1.3k
Vibe coding コードレビュー
kinopeee
0
450
STUNMESH-go: Wireguard NAT穿隧工具的源起與介紹
tjjh89017
0
380
DynamoDBは怖くない!〜テーブル設計の勘所とテスト戦略〜
hyamazaki
1
200
GUI操作LLMの最新動向: UI-TARSと関連論文紹介
kfujikawa
0
960
Featured
See All Featured
Unsuck your backbone
ammeep
671
58k
Art, The Web, and Tiny UX
lynnandtonic
301
21k
The Myth of the Modular Monolith - Day 2 Keynote - Rails World 2024
eileencodes
26
3k
We Have a Design System, Now What?
morganepeng
53
7.7k
CoffeeScript is Beautiful & I Never Want to Write Plain JavaScript Again
sstephenson
161
15k
Building Better People: How to give real-time feedback that sticks.
wjessup
367
19k
Scaling GitHub
holman
462
140k
Stop Working from a Prison Cell
hatefulcrawdad
271
21k
RailsConf & Balkan Ruby 2019: The Past, Present, and Future of Rails at GitHub
eileencodes
139
34k
Automating Front-end Workflow
addyosmani
1370
200k
The Straight Up "How To Draw Better" Workshop
denniskardys
236
140k
Practical Tips for Bootstrapping Information Extraction Pipelines
honnibal
PRO
23
1.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