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 review than you might think
Search
Daniel Shaw
October 21, 2017
Programming
1
320
There's more to code review than you might think
Given at DrupalCamp Dublin 2017
Daniel Shaw
October 21, 2017
Tweet
Share
More Decks by Daniel Shaw
See All by Daniel Shaw
Educating Enfys (lightning talk)
thatdamnqa
0
260
There's More to Code Reviews than You Might Think ✩
thatdamnqa
0
110
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
270
The Less Dull Bits of Testing
thatdamnqa
0
250
Other Decks in Programming
See All in Programming
Reactive Thinking with Signals and the Resource API
manfredsteyer
PRO
0
110
CSC509 Lecture 08
javiergs
PRO
0
250
Catch Up: Go Style Guide Update
andpad
0
250
テーブル定義書の構造化抽出して、生成AIでDWH分析を試してみた / devio2025tokyo
kasacchiful
0
280
Migration to Signals, Resource API, and NgRx Signal Store
manfredsteyer
PRO
0
110
What Spring Developers Should Know About Jakarta EE
ivargrimstad
0
460
O Que É e Como Funciona o PHP-FPM?
marcelgsantos
0
190
GC25 Recap: The Code You Reviewed is Not the Code You Built / #newt_gophercon_tour
mazrean
0
110
What's new in Spring Modulith?
olivergierke
1
170
エンジニアインターン「Treasure」とHonoの2年、そして未来へ / Our Journey with Hono Two Years at Treasure and Beyond
carta_engineering
0
420
Android16 Migration Stories ~Building a Pattern for Android OS upgrades~
reoandroider
0
130
開発組織の戦略的な役割と 設計スキル向上の効果
masuda220
PRO
9
1.5k
Featured
See All Featured
Done Done
chrislema
185
16k
Designing Dashboards & Data Visualisations in Web Apps
destraynor
231
53k
Refactoring Trust on Your Teams (GOTO; Chicago 2020)
rmw
35
3.2k
Become a Pro
speakerdeck
PRO
29
5.6k
Dealing with People You Can't Stand - Big Design 2015
cassininazir
367
27k
実際に使うSQLの書き方 徹底解説 / pgcon21j-tutorial
soudai
PRO
190
55k
I Don’t Have Time: Getting Over the Fear to Launch Your Podcast
jcasabona
34
2.5k
Building a Scalable Design System with Sketch
lauravandoore
463
33k
Creating an realtime collaboration tool: Agile Flush - .NET Oxford
marcduiker
34
2.3k
Visualization
eitanlees
149
16k
The Pragmatic Product Professional
lauravandoore
36
7k
Templates, Plugins, & Blocks: Oh My! Creating the theme that thinks of everything
marktimemedia
31
2.6k
Transcript
THERE’S MORE TO CODE REVIEW THAN YOU MIGHT THINK Daniel
Shaw @thatdamnqa · http://thatdamnqa.com
@thatdamnqa What are Code reviews for?
@thatdamnqa Code reviews are not an indication of anybody's abilities
@thatdamnqa ☐ 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
@thatdamnqa Check code style
@thatdamnqa Check code style Pick a house style, use it,
communicate it
@thatdamnqa Check code style Pick a house style, use it,
communicate it
@thatdamnqa Check code style Don't get too distracted by checking
code style
@thatdamnqa Check code style Automate if you can. Bad news
can be better received by a cruel and lifeless script
@thatdamnqa Review the configuration
@thatdamnqa Review the configuration { "require": { "usefultool/tool": "1.1" }
}
@thatdamnqa Review the configuration { "require": { "usefultool/tool": "1.2.1" }
}
@thatdamnqa Review the configuration { "require": { "usefultool/tool": "^1.2" }
}
@thatdamnqa Check forwards compatibility
@thatdamnqa Check forwards compatibility PHP 5.6 PHP 7.0 PHP 7.1
@thatdamnqa Check forwards compatibility jQuery 1 jQuery 2
@thatdamnqa Check forwards compatibility jQuery 1 jQuery 2 jQuery 3
@thatdamnqa
@thatdamnqa Review the documentation
@thatdamnqa Review the documentation Ensure changes make sense, and are
correct
@thatdamnqa Review the documentation Make sure code changes are documented
@thatdamnqa Review the commit message
@thatdamnqa Review the commit message Update usefultool.
@thatdamnqa 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]
@thatdamnqa Double check the code
@thatdamnqa Double check the code if (in_array($user_id, $permitted_users)) { $this->showRestrictedSecrets();
} else { $this->accessDenied(); }
@thatdamnqa Double check the code $permitted = ['2', '4', '5',
'cake']; $input = 0; if (in_array($input, $permitted)) { echo "I'm in!"; }
@thatdamnqa Double check the code Is the code testable?
@thatdamnqa Double check the code if (in_array($user_id, $permitted_users)) { $this->showRestrictedSecrets();
} else { $this->accessDenied(); }
@thatdamnqa Double check the code function is_allowed_access($input, $expected) { if
(in_array($input, $expected), true) { return true; } else { return false; } }
@thatdamnqa 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 ); }
@thatdamnqa Double check the tests
@thatdamnqa Double check the tests Test for failures too
@thatdamnqa 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 ); }
@thatdamnqa 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 ); }
@thatdamnqa Final thoughts…
@thatdamnqa Final thoughts… All output should be reviewed. It’s not
personal
@thatdamnqa Final thoughts… Often, a peer review can teach something
to two people
@thatdamnqa Introducing code reviews
@thatdamnqa Introducing Code Reviews Step 1: Keep watching
@thatdamnqa Introducing Code Reviews Step 1: Keep watching Step 2:
Ask questions
@thatdamnqa Introducing Code Reviews Step 1: Keep watching Step 2:
Ask questions Step 3: Go to step 1
@thatdamnqa
@thatdamnqa ☑ 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]
@thatdamnqa