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 Think ✩
Search
Daniel Shaw
October 14, 2016
Programming
0
93
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
230
Educating Enfys (lightning talk)
thatdamnqa
0
190
There's more to code review than you might think
thatdamnqa
0
160
There's More To Code Reviews Than You Might Think (lightning talk)
thatdamnqa
0
180
The Less Dull Bits of Testing
thatdamnqa
0
150
Other Decks in Programming
See All in Programming
Komplexe Oberflächen mit SVG und der Web Animation API
joergneumann
0
670
『Railsオワコン』と言われる時代に、なぜブルーモ証券はRailsを選ぶのか
free_world21
0
250
新宿ダンジョンを可視化してみた
satoshi7190
2
260
Milestoner
bkuhlmann
1
410
コーンフレークから始める モデリング会話入門
ogurotakayuki
0
370
Let's learn code review
riofujimon
2
410
Netty Chicago Java User Group 2024-04-17
sullis
0
180
使ってみよう Azure AI Document Intelligence
kosmosebi
2
320
Azure OpenAI Serviceのプロンプトエンジニアリング入門
tomokusaba
3
710
Site Reliability Engineering for GMO
pyama86
8
1k
大規模UIKitベースアプリへのTCAの段階的導入/gradual-adoption-of-tca-in-a-large-scale-uikit-based-app
takehilo
1
180
GraphQLサーバの構成要素を整理する #ハッカー鮨 #tsukijigraphql / graphql server technology selection
izumin5210
4
840
Featured
See All Featured
The Straight Up "How To Draw Better" Workshop
denniskardys
227
130k
[RailsConf 2023 Opening Keynote] The Magic of Rails
eileencodes
9
8.3k
"I'm Feeling Lucky" - Building Great Search Experiences for Today's Users (#IAC19)
danielanewman
221
21k
The Pragmatic Product Professional
lauravandoore
25
5.8k
Practical Orchestrator
shlominoach
182
9.7k
What's new in Ruby 2.0
geeforr
337
31k
Refactoring Trust on Your Teams (GOTO; Chicago 2020)
rmw
25
2.3k
The Language of Interfaces
destraynor
151
23k
Building a Modern Day E-commerce SEO Strategy
aleyda
17
6.4k
Fontdeck: Realign not Redesign
paulrobertlloyd
76
4.9k
Designing with Data
zakiwarfel
96
4.8k
The Invisible Customer
myddelton
114
12k
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