1
00:00:00,040 --> 00:00:04,320
Welcome back to the Deep Dive. 
Today, we're opening up the hood

2
00:00:04,560 --> 00:00:06,840
on Code Review. 
It's a practice that's just 

3
00:00:06,840 --> 00:00:09,440
fundamental to modern software 
development, isn't it? 

4
00:00:09,440 --> 00:00:12,880
But you know, it often feels 
like this really intense, super 

5
00:00:12,880 --> 00:00:15,000
specific kind of conversation 
you have at work. 

6
00:00:15,080 --> 00:00:17,560
Oh absolutely. 
It's probably the single most 

7
00:00:17,560 --> 00:00:20,160
effective safety mechanism any 
software team has. 

8
00:00:20,160 --> 00:00:23,080
And we're not just talking like 
catching typos. 

9
00:00:23,080 --> 00:00:27,440
We're talking about the real, 
medium and long term health of a

10
00:00:27,440 --> 00:00:30,200
systems code base. 
Maintainability. 

11
00:00:30,240 --> 00:00:32,439
Yeah. 
And it's not niche anymore, is 

12
00:00:32,439 --> 00:00:33,720
it? 
This is pretty much everywhere. 

13
00:00:33,760 --> 00:00:35,840
It really is. 
It's standard practice. 

14
00:00:35,840 --> 00:00:37,960
I mean, the data backs that up 
quite strongly. 

15
00:00:38,040 --> 00:00:40,160
Right, you mentioned some data, 
the Stack Overflow survey. 

16
00:00:40,160 --> 00:00:43,120
Yeah exactly. 
Back in 2019 they surveyed 

17
00:00:43,640 --> 00:00:47,040
something like over 70,000 
developers and the results were 

18
00:00:47,040 --> 00:00:51,600
pretty start only about 23.6%, 
so less than 1/4 said they 

19
00:00:51,600 --> 00:00:54,440
didn't use code review in their 
day-to-day work. 

20
00:00:54,960 --> 00:00:57,760
Wow, so more than 3/4 of 
developers rely on this? 

21
00:00:57,760 --> 00:00:58,800
It's huge. 
It is. 

22
00:00:58,800 --> 00:01:01,360
It shows how critical this 
feedback loop is considered 

23
00:01:01,360 --> 00:01:03,680
across the industry. 
OK, that sets the stage 

24
00:01:03,680 --> 00:01:06,840
perfectly. 
So our mission today then is to 

25
00:01:06,840 --> 00:01:08,800
do a deep dive into the source 
material. 

26
00:01:08,800 --> 00:01:12,800
We looked at specifically a 14 
code review by Marco, Tulio 

27
00:01:12,800 --> 00:01:17,160
Valente and Aline Torres. 
We want to pull out the the core

28
00:01:17,160 --> 00:01:21,840
mechanics, the reasons behind 
it, the motivations, and maybe 

29
00:01:21,840 --> 00:01:25,160
most importantly, the practical 
best practices for both sides, 

30
00:01:25,160 --> 00:01:27,880
right reviewers and authors. 
That sounds like a plan. 

31
00:01:28,000 --> 00:01:29,880
Where should we start the basic 
process? 

32
00:01:29,880 --> 00:01:32,800
Yeah, let's start there. 
Yeah, fundamentally, the concept

33
00:01:32,800 --> 00:01:36,080
seems simple enough. 
A developer writes some code and

34
00:01:36,080 --> 00:01:39,120
before it gets merged into the 
main system, at least one other 

35
00:01:39,120 --> 00:01:41,600
developer, the reviewer, has to 
inspect it. 

36
00:01:41,920 --> 00:01:43,720
Right. 
And that inspection kicks off 

37
00:01:43,720 --> 00:01:45,400
this well, this established 
dialogue. 

38
00:01:45,400 --> 00:01:47,160
It's got a structure. 
It usually starts when the 

39
00:01:47,160 --> 00:01:49,280
reviewer adds comments to the 
code changes. 

40
00:01:49,280 --> 00:01:50,520
Those comments can be about 
anything. 

41
00:01:50,600 --> 00:01:51,320
Really. 
Pretty much. 

42
00:01:51,760 --> 00:01:53,920
They might be asking for 
clarification, like why did you 

43
00:01:53,920 --> 00:01:56,560
do it this way, or suggesting 
improvements may be a better way

44
00:01:56,560 --> 00:01:59,120
to structure something. 
Or, you know, the big one, 

45
00:01:59,400 --> 00:02:02,240
identifying an actual bug. 
OK, so the review happens, 

46
00:02:02,240 --> 00:02:04,760
comments are made, then the 
interaction really starts. 

47
00:02:04,760 --> 00:02:07,600
The author gets that feedback. 
Exactly, and they have to 

48
00:02:07,600 --> 00:02:11,240
respond professionally. 
There are basically 2 paths. 

49
00:02:11,760 --> 00:02:15,680
Either they take the feedback on
board, modify their code based 

50
00:02:15,680 --> 00:02:19,800
on the suggestions, or they 
reply and justify their original

51
00:02:19,800 --> 00:02:21,760
approach. 
Maybe there's a good reason it 

52
00:02:21,760 --> 00:02:23,960
was done that way. 
It's a conversation. 

53
00:02:23,960 --> 00:02:25,920
A negotiation, Almost. 
Kind of. 

54
00:02:25,960 --> 00:02:28,560
And that back and forth 
continues until the reviewer is 

55
00:02:28,560 --> 00:02:31,600
satisfied they give their 
approval, and only then does the

56
00:02:31,600 --> 00:02:34,320
code actually get integrated. 
And we see this happening all 

57
00:02:34,320 --> 00:02:35,880
the time in tools like GitHub, 
right? 

58
00:02:36,320 --> 00:02:39,320
With pull requests or PRS. 
Yeah, PRS are the common 

59
00:02:39,320 --> 00:02:43,920
mechanism, and our sources had a
great real world example from 

60
00:02:43,920 --> 00:02:47,040
APR on the Microsoft Disc Code 
Project Big. 

61
00:02:47,040 --> 00:02:48,040
Project. 
Oh yeah, huge. 

62
00:02:48,480 --> 00:02:52,000
What does that example show? 
It really highlighted the, let's

63
00:02:52,000 --> 00:02:53,800
say, the delicate balance you 
need. 

64
00:02:53,800 --> 00:02:56,200
In this conversation, the 
reviewer did something 

65
00:02:56,200 --> 00:02:58,040
interesting. 
They started with praise, 

66
00:02:58,280 --> 00:03:01,360
congratulated the author, said 
the solution was elegant. 

67
00:03:01,520 --> 00:03:03,760
That's important, isn't it? 
Starting positive. 

68
00:03:04,040 --> 00:03:08,360
Crucial builds trust, makes the 
critique easier to swallow. 

69
00:03:09,040 --> 00:03:11,160
Because then they switched 
gears. 

70
00:03:11,480 --> 00:03:13,280
OK, they pointed out several 
issues. 

71
00:03:13,280 --> 00:03:17,840
Very specifically, things like 
an internal design problem 

72
00:03:17,840 --> 00:03:19,760
exposing a method that shouldn't
be public. 

73
00:03:20,480 --> 00:03:23,360
Technical stuff. 
Right, but also smaller things 

74
00:03:23,400 --> 00:03:28,040
like layout issues, colors, tab 
alignment, code hygiene. 

75
00:03:28,760 --> 00:03:30,680
OK, so big picture in small 
details. 

76
00:03:30,680 --> 00:03:34,520
Exactly, and then this was key 
when they found an actual bug. 

77
00:03:34,520 --> 00:03:37,600
They didn't just say this is 
broken, they attached a video 

78
00:03:37,760 --> 00:03:39,600
showing how to reproduce it. 
Oh nice. 

79
00:03:39,640 --> 00:03:41,040
That's helpful. 
Super helpful. 

80
00:03:41,040 --> 00:03:44,560
So you see that mix praise, 
specific critique covering 

81
00:03:44,560 --> 00:03:46,760
different levels and clear 
evidence for bugs. 

82
00:03:46,760 --> 00:03:49,440
It's it's serious technical 
feedback, but delivered 

83
00:03:49,440 --> 00:03:51,600
respectfully. 
OK, that nails down the how 

84
00:03:51,600 --> 00:03:54,040
pretty well, so let's move to 
the why. 

85
00:03:54,120 --> 00:03:55,840
Why do teams spend so much time 
on this? 

86
00:03:55,840 --> 00:03:57,680
I mean, finding bugs is the 
obvious one, right? 

87
00:03:57,680 --> 00:03:59,000
That's usually the first thing 
people think of. 

88
00:03:59,000 --> 00:04:01,920
Yeah. 
And it came out on top in that 

89
00:04:01,920 --> 00:04:04,560
big 2013 study by the Kelly and 
Bird. 

90
00:04:04,560 --> 00:04:07,440
Right, the Microsoft one. 
Nearly 900 developers and 

91
00:04:07,440 --> 00:04:10,080
testers surveyed. 
They asked them straight up, 

92
00:04:10,800 --> 00:04:13,320
what are your main motivations 
for doing code review? 

93
00:04:13,760 --> 00:04:16,600
And number one was finding bugs.
No surprise. 

94
00:04:16,600 --> 00:04:19,640
There no surprise at all. 
It's the immediate, tangible 

95
00:04:19,640 --> 00:04:22,480
benefit, catching problems 
before they hit production. 

96
00:04:22,480 --> 00:04:24,520
But the study found other 
reasons too, didn't it? 

97
00:04:24,520 --> 00:04:27,640
Deeper ones. 
It did, and these are arguably 

98
00:04:27,640 --> 00:04:31,200
where the like the long term 
strategic value really comes in.

99
00:04:31,680 --> 00:04:33,760
They identified three other big 
ones. 

100
00:04:33,840 --> 00:04:37,440
First, just improving the code 
overall, making it cleaner, 

101
00:04:37,440 --> 00:04:40,240
easier to maintain later. 
OK, makes sense. 

102
00:04:40,240 --> 00:04:42,600
Second, proposing alternative 
solutions. 

103
00:04:42,600 --> 00:04:45,360
Maybe there's a more efficient 
algorithm or design pattern that

104
00:04:45,360 --> 00:04:46,240
fits better? 
Right. 

105
00:04:46,320 --> 00:04:47,640
Different perspectives. 
Exactly. 

106
00:04:48,160 --> 00:04:52,040
And 3rd and this one I think is 
huge is knowledge sharing. 

107
00:04:52,800 --> 00:04:55,480
OK, let's unpack that knowledge 
sharing. 

108
00:04:55,920 --> 00:04:59,320
How does that work in review? 
Well, it flows both ways. 

109
00:04:59,320 --> 00:05:01,240
You see, the author learns from 
the reviewer. 

110
00:05:01,240 --> 00:05:03,600
Maybe the reviewer has more 
experience or knows that part of

111
00:05:03,600 --> 00:05:06,520
the system better. 
But the reviewer also learns 

112
00:05:06,800 --> 00:05:08,320
they have to understand the new 
code. 

113
00:05:08,320 --> 00:05:11,080
Maybe it touches a part of the 
system they weren't familiar 

114
00:05:11,080 --> 00:05:13,760
with, or uses a technique they 
haven't seen. 

115
00:05:13,840 --> 00:05:17,480
So it stops knowledge getting 
siloed like preventing those 

116
00:05:17,480 --> 00:05:19,160
knowledge islands people talk 
about. 

117
00:05:19,200 --> 00:05:21,480
Precisely. 
Those islands are so dangerous 

118
00:05:21,480 --> 00:05:24,000
in software projects. 
Yeah, where only one person 

119
00:05:24,000 --> 00:05:26,080
understands how something 
critical works. 

120
00:05:26,080 --> 00:05:30,200
And if that person leaves or is 
unavailable, you're stuck. 

121
00:05:30,560 --> 00:05:33,280
Major risk often called the bus 
factor, right? 

122
00:05:33,800 --> 00:05:36,320
How many people need to get hit 
by a bus before the project 

123
00:05:36,320 --> 00:05:36,960
stalls? 
Grim. 

124
00:05:37,400 --> 00:05:39,920
But yeah, code review directly 
fights that. 

125
00:05:40,280 --> 00:05:41,720
It forces knowledge to be 
shared. 

126
00:05:42,080 --> 00:05:44,560
Multiple people see the code, 
understand the logic. 

127
00:05:44,760 --> 00:05:47,560
It makes the whole team and the 
project more resilient. 

128
00:05:47,760 --> 00:05:50,840
OK, that makes the strategic 
value really clear. 

129
00:05:51,360 --> 00:05:55,480
Finding bugs is just the start. 
It's about quality, alternatives

130
00:05:55,480 --> 00:05:57,280
and, crucially, spreading 
knowledge. 

131
00:05:57,280 --> 00:05:59,400
Exactly. 
All right, so now let's imagine 

132
00:05:59,400 --> 00:06:01,000
I'm the reviewer. 
I've got APR in front of me. 

133
00:06:01,280 --> 00:06:03,280
What specifically should I be 
looking for? 

134
00:06:03,280 --> 00:06:05,000
What's on the the checklist 
beyond just? 

135
00:06:05,160 --> 00:06:07,080
Does it work? 
It's a pretty comprehensive 

136
00:06:07,080 --> 00:06:09,520
checklist actually. 
You start with the basics, of 

137
00:06:09,520 --> 00:06:12,000
course. 
Bugs and functionality does it 

138
00:06:12,000 --> 00:06:14,760
do what it claims to do. 
But you need to go deeper. 

139
00:06:15,160 --> 00:06:20,600
Think about concurrency 
problems, race conditions, 

140
00:06:20,920 --> 00:06:23,840
deadlocks, things that might 
only show up under load. 

141
00:06:24,040 --> 00:06:26,160
Tricky stuff. 
Yeah, and error handling. 

142
00:06:26,160 --> 00:06:29,320
Are exceptions caught properly? 
Are error messages useful? 

143
00:06:29,320 --> 00:06:31,720
That sort of thing. 
OK, so functional correctness 

144
00:06:31,720 --> 00:06:35,120
and robustness. 
What about the codes structure, 

145
00:06:35,520 --> 00:06:37,720
The design? 
Hugely important for the long 

146
00:06:37,720 --> 00:06:39,240
term. 
You're looking for code that's, 

147
00:06:39,520 --> 00:06:42,440
well, overly complex. 
Code that's hard to read, hard 

148
00:06:42,440 --> 00:06:44,680
to understand. 
That's a major red flag for 

149
00:06:44,680 --> 00:06:47,640
future maintenance. 
Complexity kills. 

150
00:06:47,680 --> 00:06:49,840
It really does. 
Also, does it violate any 

151
00:06:49,840 --> 00:06:52,120
established design principles 
the team follows? 

152
00:06:52,320 --> 00:06:54,800
SOLID principles maybe? 
Or does it clash with the 

153
00:06:54,800 --> 00:06:57,040
overall system architecture? 
You need to protect that 

154
00:06:57,040 --> 00:06:58,960
architecture. 
OK, design principles, 

155
00:06:58,960 --> 00:07:00,440
architecture. 
What about efficiency? 

156
00:07:01,080 --> 00:07:02,600
Performance. 
Yeah, that's on the list too. 

157
00:07:02,800 --> 00:07:05,120
Is the author using 
fundamentally inefficient 

158
00:07:05,120 --> 00:07:08,320
algorithms or data structures 
Like scanning a huge list 

159
00:07:08,320 --> 00:07:11,000
repeatedly when a hash map would
be instant, right? 

160
00:07:11,640 --> 00:07:15,080
Obvious performance bottlenecks.
Obvious ones, yes, but you also 

161
00:07:15,080 --> 00:07:18,200
need to watch out for the 
opposite premature optimization.

162
00:07:19,240 --> 00:07:20,840
Optimizing code that doesn't 
need it. 

163
00:07:20,840 --> 00:07:24,000
Exactly. 
Spending ages making some rarely

164
00:07:24,000 --> 00:07:27,840
used function a tiny bit faster 
when that time could be spent on

165
00:07:27,840 --> 00:07:30,120
features or fixing real 
bottlenecks. 

166
00:07:30,240 --> 00:07:31,800
It's a balance. 
Got it. 

167
00:07:32,320 --> 00:07:35,920
And then there's the the hygiene
factors, the small stuff. 

168
00:07:35,920 --> 00:07:38,240
Yeah, code hygiene. 
This is where things like code 

169
00:07:38,240 --> 00:07:41,120
smells come in. 
Code smells for listeners. 

170
00:07:41,120 --> 00:07:43,560
That's not a literal bug, right?
It's more like a warning sign. 

171
00:07:43,560 --> 00:07:45,560
Precisely. 
It's a symptom in the code that 

172
00:07:45,560 --> 00:07:48,120
suggests A deeper problem might 
exist in the design or 

173
00:07:48,120 --> 00:07:50,240
structure. 
Things like methods that are way

174
00:07:50,240 --> 00:07:54,520
too long, or lots of duplicated 
code, or classes doing too many 

175
00:07:54,520 --> 00:07:57,400
different things. 
Okay, this category also covers 

176
00:07:57,400 --> 00:07:59,880
the basics like are they 
following the teams naming 

177
00:07:59,880 --> 00:08:02,360
conventions? 
Is the indentation and layout 

178
00:08:02,360 --> 00:08:04,440
consistent? 
Readability matters. 

179
00:08:04,640 --> 00:08:06,640
Absolutely. 
And finally, what about the sort

180
00:08:06,640 --> 00:08:09,440
of safety net stuff? 
Tests, documentation, security. 

181
00:08:09,440 --> 00:08:12,200
Essential checks? 
Is there adequate test coverage,

182
00:08:12,600 --> 00:08:15,800
or maybe no tests at all? 
Is complex logic properly 

183
00:08:15,800 --> 00:08:20,200
documented with comments and 
crucially, security? 

184
00:08:20,840 --> 00:08:24,120
Are there potential 
vulnerabilities, improper 

185
00:08:24,120 --> 00:08:28,480
handling of sensitive data, 
privacy issues, and even things 

186
00:08:28,480 --> 00:08:32,960
like are they using third party 
libraries or APIs correctly, or 

187
00:08:32,960 --> 00:08:35,480
are they pulling in some 
unauthorized library that could 

188
00:08:35,480 --> 00:08:38,159
cause licensing headaches or 
security risks later? 

189
00:08:38,440 --> 00:08:40,960
It's a lot. 
The reviewer is really acting as

190
00:08:40,960 --> 00:08:43,159
a gatekeeper for overall system 
health. 

191
00:08:43,280 --> 00:08:45,600
That's a good way to put it, A 
holistic gatekeeper. 

192
00:08:45,600 --> 00:08:47,640
Which brings us to the human 
element. 

193
00:08:47,640 --> 00:08:52,320
Yeah, because reviewing based on
that huge checklist, well, it 

194
00:08:52,320 --> 00:08:55,680
requires tact, doesn't it? 
If the feedback isn't delivered 

195
00:08:55,680 --> 00:08:57,600
well, the whole process can 
break down. 

196
00:08:57,720 --> 00:09:00,160
Completely. 
This is where soft skills become

197
00:09:00,160 --> 00:09:01,880
just as important as technical 
skills. 

198
00:09:02,040 --> 00:09:05,920
Michaela Greylers work analyzing
hundreds of GitHub comments gave

199
00:09:05,920 --> 00:09:07,320
some great pointers here. 
Okay. 

200
00:09:07,320 --> 00:09:09,400
What were the key takeaways for 
reviewers? 

201
00:09:09,800 --> 00:09:12,000
First rule is probably 
restraint, especially with 

202
00:09:12,000 --> 00:09:14,240
alternatives. 
Only suggest a different way if 

203
00:09:14,240 --> 00:09:17,120
it's clearly objectively better,
not just because you would have 

204
00:09:17,120 --> 00:09:19,120
written it differently. 
That's a tough one, resisting 

205
00:09:19,120 --> 00:09:21,040
the urge to impose personal 
style. 

206
00:09:21,320 --> 00:09:23,120
It is. 
You have to avoid purely 

207
00:09:23,120 --> 00:09:26,600
subjective comments, like 
arguing about variable names, 

208
00:09:26,960 --> 00:09:29,480
unless they're genuinely 
confusing or violate 

209
00:09:29,480 --> 00:09:32,040
conventions. 
Focus on substance. 

210
00:09:32,080 --> 00:09:35,240
And professionalism in tone is 
paramount, I assume. 

211
00:09:35,240 --> 00:09:38,760
Absolutely no offensive 
language, no sarcasm, no irony. 

212
00:09:38,840 --> 00:09:41,720
Keep it professional and keep 
the comments strictly about the 

213
00:09:41,720 --> 00:09:43,800
code. 
No personal stuff, obviously. 

214
00:09:43,800 --> 00:09:48,360
OK, so maintain professionalism.
What about specific tactics to 

215
00:09:48,360 --> 00:09:52,000
make the feedback constructive? 
The sources mentioned framing 

216
00:09:52,000 --> 00:09:54,520
things as questions. 
Yes, that's a great technique. 

217
00:09:54,640 --> 00:09:56,600
It softens the critique 
immediately. 

218
00:09:56,600 --> 00:09:59,720
Instead of saying this variable 
is useless, you ask, hey, is 

219
00:09:59,720 --> 00:10:01,120
this variable actually used 
anywhere? 

220
00:10:01,120 --> 00:10:03,680
Maybe we can remove it? 
Right, opens a dialogue instead 

221
00:10:03,680 --> 00:10:05,440
of issuing a command. 
Exactly. 

222
00:10:05,720 --> 00:10:09,440
And another powerful one is 
using inclusive language we 

223
00:10:09,480 --> 00:10:11,120
instead of you. 
How does that help? 

224
00:10:11,520 --> 00:10:14,280
It reinforces that you're a team
working together on the same 

225
00:10:14,280 --> 00:10:16,040
code. 
So instead of you should make 

226
00:10:16,040 --> 00:10:18,120
this private track. 
Could we make this attribute 

227
00:10:18,120 --> 00:10:20,440
private? 
Might encapsulate things better.

228
00:10:20,680 --> 00:10:22,760
Small change, big difference in 
feel. 

229
00:10:22,840 --> 00:10:24,800
Yeah, I can see that more 
collaborative. 

230
00:10:24,960 --> 00:10:28,000
Definitely also back up your 
comments if you suggest a 

231
00:10:28,000 --> 00:10:30,440
change, especially a non obvious
one. 

232
00:10:30,440 --> 00:10:34,440
Provide a reason linked to 
documentation, internal style 

233
00:10:34,440 --> 00:10:37,600
guides, external articles, 
justify. 

234
00:10:37,680 --> 00:10:40,000
It and that justification is 
even more important for junior 

235
00:10:40,000 --> 00:10:41,920
developers, right? 
Yeah, It becomes a teaching 

236
00:10:41,920 --> 00:10:43,880
moment. 
Absolutely don't just tell a 

237
00:10:43,880 --> 00:10:46,120
junior dev. 
Use a hash map here, not an 

238
00:10:46,120 --> 00:10:49,000
array list. 
Explain why for this kind of 

239
00:10:49,000 --> 00:10:51,840
data access, a hash map gives us
much faster lookups when the 

240
00:10:51,840 --> 00:10:54,200
data set gets large. 
Teach the reasoning. 

241
00:10:54,200 --> 00:10:56,360
OK, justify suggestions. 
What else? 

242
00:10:56,480 --> 00:10:59,040
Positive feedback. 
Yes, don't forget praise. 

243
00:10:59,200 --> 00:11:01,880
If the code is well written or 
the tests are really thorough, 

244
00:11:01,920 --> 00:11:05,200
say so. 
A simple nice work on the clear 

245
00:11:05,200 --> 00:11:07,200
test setup made review much 
easier. 

246
00:11:07,200 --> 00:11:09,640
Goes a long way. 
It balances the critique. 

247
00:11:09,640 --> 00:11:11,120
Guilds Goodwill. 
Totally. 

248
00:11:11,480 --> 00:11:14,160
And finally, humility. 
As a reviewer, you will be 

249
00:11:14,160 --> 00:11:16,040
wrong. 
Sometimes you'll misunderstand 

250
00:11:16,040 --> 00:11:18,080
something. 
When the author corrects you 

251
00:11:18,160 --> 00:11:21,720
just acknowledge it gracefully. 
OK, I see what you mean now. 

252
00:11:21,720 --> 00:11:24,280
Thanks for explaining. 
No need to get defensive. 

253
00:11:24,280 --> 00:11:26,800
And what about disagreements 
when you just can't agree 

254
00:11:26,800 --> 00:11:28,560
through comments? 
That's when you might need a 

255
00:11:28,560 --> 00:11:31,760
synchronous meeting like a quick
call, but that should be rare, 

256
00:11:32,400 --> 00:11:35,600
really reserved for significant 
disagreements, or when you're 

257
00:11:35,600 --> 00:11:38,120
just not converging on approval 
via comments. 

258
00:11:38,360 --> 00:11:41,040
Make it the exception. 
OK, that's a great set of 

259
00:11:41,040 --> 00:11:43,640
guidelines for the reviewer. 
Now let's flip the coin. 

260
00:11:44,320 --> 00:11:47,080
What about the author? 
The person receiving all the 

261
00:11:47,080 --> 00:11:48,800
scrutiny? 
What's the advice for them? 

262
00:11:49,000 --> 00:11:51,120
The first thing for the author 
is mindset. 

263
00:11:51,480 --> 00:11:55,680
Be professional, be polite, but 
crucially understand that code 

264
00:11:55,680 --> 00:12:00,320
review isn't like a test of your
overall programming skill. 

265
00:12:00,320 --> 00:12:02,560
Right, it's about the specific 
code submitted. 

266
00:12:02,560 --> 00:12:04,040
Exactly. 
Don't take the feedback 

267
00:12:04,040 --> 00:12:05,920
personally. 
It's about making this piece of 

268
00:12:05,920 --> 00:12:09,000
code Better Together. 
OK, mindset is key. 

269
00:12:09,280 --> 00:12:12,920
What about practical tactics for
the author to make the process 

270
00:12:12,920 --> 00:12:15,320
smoother? 
There's 1 golden rule, maybe the

271
00:12:15,320 --> 00:12:18,640
most important piece of advice 
for authors who want good fast 

272
00:12:18,640 --> 00:12:22,480
reviews, which is submit small 
pool requests, small PRS, ah. 

273
00:12:23,320 --> 00:12:26,040
Keep the changes focused and 
limited in size. 

274
00:12:26,160 --> 00:12:29,080
Why is that so critical? 
Because reviewers are human. 

275
00:12:29,440 --> 00:12:32,280
The authors of Software 
Engineering at Google are really

276
00:12:32,280 --> 00:12:34,880
clear on this. 
They recommend APR should 

277
00:12:34,880 --> 00:12:39,720
ideally be no more than say 200 
lines of code. 200 lines that 

278
00:12:39,720 --> 00:12:41,720
sounds manageable to review 
properly. 

279
00:12:41,720 --> 00:12:43,600
It is. 
It lets the reviewer focus, 

280
00:12:43,640 --> 00:12:46,680
understand the context and 
actually find subtle issues. 

281
00:12:46,960 --> 00:12:50,080
When PRS get huge, quality just 
plummets. 

282
00:12:50,160 --> 00:12:52,320
Yeah, there's that great quote. 
Wasn't there something about 

283
00:12:52,320 --> 00:12:56,800
reviewing 20 lines versus 500? 
Oh yeah, the tweet cited in the 

284
00:12:56,800 --> 00:12:58,600
sources. 
It sums it up perfectly. 

285
00:12:58,880 --> 00:13:01,240
Ask a programmer to review 20 
lines of code. 

286
00:13:01,240 --> 00:13:04,880
They'll find 7 issues. 
Ask them to review 500 lines and

287
00:13:04,880 --> 00:13:08,520
they'll find 0 issues. 
Yeah, it feels true, though 

288
00:13:08,600 --> 00:13:11,400
reviewer fatigue is real. 
It absolutely is. 

289
00:13:11,400 --> 00:13:13,760
If you dump 1000 lines on 
someone, they're just going to 

290
00:13:13,760 --> 00:13:15,600
skim it. 
They might catch obvious stuff, 

291
00:13:15,600 --> 00:13:18,320
but the tricky bugs, the design 
flaws, they'll likely slip right

292
00:13:18,320 --> 00:13:21,120
through. 
Small PRS force focus. 

293
00:13:21,120 --> 00:13:25,200
So small PRS mean better quality
reviews, faster integration, and

294
00:13:25,200 --> 00:13:28,080
ultimately less risk going into 
the main code base. 

295
00:13:28,360 --> 00:13:29,600
Couldn't have said it better 
myself. 

296
00:13:30,400 --> 00:13:33,440
And beyond size, authors also 
need to write clear PR 

297
00:13:33,440 --> 00:13:36,600
descriptions, explain what the 
change does and why it's needed,

298
00:13:36,840 --> 00:13:40,920
give the reviewer context. 
And be prepared to justify 

299
00:13:40,920 --> 00:13:44,720
decisions if asked, just like 
they expect reviewers to justify

300
00:13:44,720 --> 00:13:45,960
suggestions. 
Exactly. 

301
00:13:45,960 --> 00:13:48,880
It's a two way St. of 
professional justification and 

302
00:13:48,880 --> 00:13:50,120
explanation. 
OK. 

303
00:13:50,600 --> 00:13:54,680
So if we zoom back out, the big 
picture here is that code review

304
00:13:54,680 --> 00:13:57,800
isn't just a gatekeeping step, 
it's this really vital 

305
00:13:57,800 --> 00:14:00,960
collaborative dialogue. 
It ensures the code itself is 

306
00:14:00,960 --> 00:14:04,920
robust, yes, but it also drives 
that crucial knowledge sharing, 

307
00:14:04,920 --> 00:14:08,120
keeps the team aligned, and 
ultimately keeps the whole 

308
00:14:08,120 --> 00:14:10,760
system healthy. 
But it relies on everyone 

309
00:14:10,760 --> 00:14:12,840
approaching it with 
professionalism intact. 

310
00:14:12,840 --> 00:14:15,280
That's it in a nutshell. 
It's really the engine room of 

311
00:14:15,280 --> 00:14:17,560
high quality sustainable 
software development. 

312
00:14:17,920 --> 00:14:20,320
Get it right and so many other 
things fall into place. 

313
00:14:20,320 --> 00:14:23,280
Get it wrong and you're storing 
up problems for the future well.

314
00:14:23,280 --> 00:14:26,240
Put a critical process that 
needs care and attention from 

315
00:14:26,240 --> 00:14:28,600
everyone involved. 
Thank you for joining us for 

316
00:14:28,600 --> 00:14:29,280
this deep dive.