Heads up! To view this whole video, sign in with your Courses Plus account or enroll in your free 7-day trial. Sign In Enroll
Preview
Start a free Courses trial
to watch this video
In this episode, we talk with Nicolas Hampton, a member of the Techdegree Success team at Treehouse, who has reviewed more than 1,000 student projects.
Mentioned References
- Steve McConnell - Code Complete 2
- 3 Misunderstandings about Code Review That Are Slowing You Down - Nicolas Hampton
- Git
- GitHub - About Pull Requests
- ESLint
- User Story
Other Resoures
Welcome to the Dev Team Show
my name is James.
0:00
In this episode,
we're gonna talk about code reviews.
0:02
Joining me is Nicholas Hampton.
0:09
Nicholas is a member of Treehouse's
technical success team.
0:11
Welcome to the show, Nicholas.
0:15
>> Thanks, James.
0:17
>> So I wanted to start just by kinda
giving a basic definition of what a code
0:19
review is just to make sure that
we're all like on the same page.
0:22
>> Of course.
0:25
>> So from my perspective,
0:26
a code review has a component
of reading code, of course.
0:28
That's sort of the,
one of the review parts.
0:31
But you also need to like,
run the code typically too, right.
0:34
So you're executing whatever it
is that the code is written-
0:38
>> Yes, you've gotta unpack the project
0:41
deploy it and
try a trial run of the program,
0:44
of course check for
any errors that might occur.
0:48
>> So
there's a validation part in there too.
0:53
So you're validating the behavior
of the application and making sure,
0:55
does this thing do what
it's supposed to be doing?
0:58
>> Exactly, and making sure that
it satisfies some user stories.
1:00
>> Right, and
then you take whatever you've noticed,
1:03
whether it be positive or negative, and
you're reporting that back to the user.
1:08
And then how you do that,
1:11
there's a variety of different
ways that that's done too.
1:12
>> Yeah, but for the most part,
you wanna keep in mind that
1:15
this is somebody working on code,
that's just trying to improve it.
1:18
So you want as much as you want to
point out anything that goes wrong,
1:22
you also want to point out
anything that they're doing right.
1:25
>> Right, positive and negative comments,
not just criticism all the time.
1:28
>> Exactly.
1:33
>> Which is easy to fall into that habit.
1:33
So in terms of the value of code reviews,
if you search the web, you'll find
1:35
opinion after opinion that people are on
board with the idea of code reviews.
1:40
I don't think that that's,
an idea that's challenged very often,
1:44
but I'm gonna read
a Steve McConnell quote.
1:48
>> Yeah.
Because I think that he kinda
1:52
gets to the point of some
research that he did,
1:53
some studies or that he's paid attention
to that really drives the point home.
1:55
So Steve McConnell from his book, Code
Complete, which is an awesome book says,
2:00
software testing alone has
limited effectiveness.
2:06
The average defect detection rate
is only 25% for unit testing,
2:09
35% functional testing and
45% for integration testing.
2:13
And in contrast, the average effectiveness
of designing code inspections are 55 and
2:19
60%.
2:25
So substantially higher than like
other forms of testing that you can do
2:26
>> Those are amazing numbers and
2:31
I completely agree with those results.
2:33
But I think the focus of those numbers
are misleading and in the wrong place.
2:36
>> Interesting, so I've got a confession.
2:41
I'll just get it, I'm getting into
the habit of doing this on the show.
2:43
>> [LAUGH]
>> Get it
2:46
out of the way right off the bat.
2:47
I've probably done about everything wrong
that you can do about code reviews.
2:48
>> Father, forgive me.
2:52
>> [LAUGH] So for instance,
I have found myself in
2:54
various situations being I think I've
heard you call it the GateKeeper.
2:59
>> Yes.
3:04
>> So what code reviews meant in that
situation was that everyone who committed
3:05
code to the project and
made changes, I was reviewing that.
3:09
Furthermore it wasn't really
integrated into our process.
3:14
So I kinda would have to
review the commit history.
3:16
>> Yeah.
>> [LAUGH]
3:20
>> The project, and
3:21
be like right, I left off here on Tuesday,
it is now Thursday,
3:22
I'd better read some of these commits,
some of these check ins.
3:25
So I would start to do that and
3:29
of course the tool didn't support
providing feedback, so I'd be writing
3:31
things down in an email typically,
which is disjointed from the code.
3:36
>> A mess.
3:40
>> A complete mess and over time
>> And it wasn't like we weren't getting
3:41
any value, but gradually over time
because nothing really worked well and
3:45
it was always one direction, like me
giving feedback to members of the team,
3:50
it breaks down and we would quit doing it,
so whatever value we were getting out of
3:55
it, we stopped getting
it because we just quit.
4:00
>> Yeah, and I think a lot of
that is being able to address
4:02
code review as you would just any
part of the development workflow.
4:07
You have to create a workflow for
yourself.
4:12
And GitHub does that really well in terms
of you could that daily peer reviews
4:15
based upon pool requests and
just going over that even as a group.
4:21
And going through and
approving each pool request.
4:26
On then, at the end of the day.
4:30
Maybe having someone take a quick look
over at, over all the poll request that
4:32
have been made just a quick look at
the reviews that were done on that.
4:36
>> Right, so real quickly poll requests,
4:40
which I think that's terminology
that others are starting to adopt.
4:42
But originally,
what I think was a GitHub idea, right?
4:45
>> Yes.
4:47
So get is the source control
that they're using, but
4:48
they're layering on this idea of a PR or
poll requests.
4:51
>> Yes.
4:54
>> And so you're working on a branch
that's separate from whatever your main
4:54
and master branch is.
4:58
>> Yes.
4:59
>> You make all your changes and
5:00
then you say hey I'm ready to merge
this upstream or into main branch.
5:02
You do a PR which basically is
sort of a fancy way of saying hey,
5:06
can you review this thing?
5:11
At the point and
time that you say, I'm ready for
5:12
a review,
Nicholas can you review this for me?
5:14
>> Yes, and a lot of times,
the beginning step of that whole
5:16
process is to fork the repo that you're
working on in the first place, so
5:22
forking the review and
making a copy of it for yourself.
5:26
Making changes to your copy and
5:30
then making a poll request to ask
whoever's overseeing this project.
5:32
Hey, I've made these changes for
these reasons, could you look it over?
5:38
And commit it if it's good, or
make comments if it needs improvement.
5:42
>> Right.
>> And it's a solid way of introducing
5:47
a workflow for your code review.
5:52
But also another problem is, I would
get annoyed if I had to do all the.
5:56
>> [LAUGH]
>> All of the pool requests for
6:02
a particular project.
6:05
Or if I was reviewing all
of those pool requests.
6:06
>> All right so
let's dig into that a little bit because,
6:08
like I just mentioned a minute ago,
6:11
that's something I've absolutely been
guilty of is being that gatekeeper person.
6:12
But in fact, to make code reviews
as effective as possible,
6:17
it should be something that's equally
shared across the entire team.
6:20
Yes, and I think guilty might
be a key word there, right?
6:23
>> [LAUGH] I wear guilt well.
6:28
>> [LAUGH] But
you're hogging all this learning material.
6:30
Everyone in your team is studying things.
6:35
>> Yep.
>> Some of them are going to overlap and
6:40
some of them are going to
be way out on a left field.
6:42
>> Right.
>> And so everyone has this
6:45
pool of knowledge to themselves.
6:48
But, when you designate one code reviewer,
6:51
that one code reviewer gets little
pieces of all that knowledge.
6:54
>> Right.
6:59
>> And then no one else dies.
7:01
>> It stops there.
7:03
>> So every team member is isolated from
every other team member's poll of notch
7:03
if you were to split that up
where peers are reviewing peers,
7:09
then like a pool of information
just puddling on itself,
7:15
we get information from everyone else.
7:19
And actually I can in that
situation slightly slow down
7:21
the amount that I'm taking
in from outside sources.
7:26
Because I'm getting all that from
the sources inside my own team.
7:29
I'm seeing all that play out in a code
base that I'm currently working on
7:33
instead of trying to decipher someone's
example that's out of context.
7:37
>> Yeah, exactly, which is great.
7:41
And it also it helps just like start those
conversations about the code you know and
7:43
people say, hey I noticed that
you did this thing over here.
7:47
Can you explain that to me and
you can use that as a starting point and
7:51
jumping off point to have some
of those deeper conversations.
7:54
>> Exactly, or even deeper than that.
7:57
I notice you're throwing
this exception error.
8:00
I don't really understand
what the wording is.
8:03
Is there a better way to
communicate to our team and
8:05
the user what this actually means.
8:08
Is this in the right format
that we wanna look at?
8:11
>> Right.
8:14
>> Even inside their own project.
8:14
>> Now, you wrote a blog post
recently about some of the current
8:16
misunderstandings or
misconceptions around code reviews.
8:19
And one of them that I thought
was really interesting was,
8:23
you talked about this expert like mindset.
8:25
So for instance over time you develop
a way or a habit of looking at things and
8:30
saying, I recognize this problem and
this is the solution that I do.
8:36
But you might miss an opportunity
to think about it differently.
8:40
And that's where if you have your
entire team involved in the code
8:42
review you made the point that someone,
maybe even with less experience,
8:46
they're actually going to have a better
chance at seeing something differently and
8:51
asking the question that says well,
have we considered this?
8:56
Did we think about that?
9:00
>> Yeah, and I think part of
that is someone who's fresh and
9:01
new to the table,
often times is often just thinking
9:06
a lot of detail about something that we've
taken for gratitude for a long time.
9:11
>> right.
>> We learned that two years ago,
9:17
I don't have to reassess that.
9:19
I'm just gonna glance over that.
9:21
That's just a meta tag.
9:23
>> Right, you can kinda short circuit
the thought process you originally went
9:24
through maybe multiple times.
9:27
To establish that pattern or
that habit and
9:29
you just sort of just cut to the chase.
9:31
And say, hey, I know how to do this.
9:32
>> Example of that,
I was trying to do a code review for
9:35
one of our students and
I kept on getting the same error.
9:39
And I was like, nothing is wrong with this
chunk of code, then I spent like a good 20
9:43
minutes on this chunk of code in for I
looked directly above that piece of code,
9:48
and they had actually written
the same piece of code
9:52
on a different configuration that was
overriding their lower piece of code,
9:56
because it was then a document on ready.
10:01
So, this was running first, then that,
and this didn't matter all.
10:04
>> Right, right.
10:07
>> It's something I would've never thought
about, because I wouldn't think that you
10:08
would have to write those two
pieces of code twice you don't.
10:11
But fresh eyes will
catch that pretty quick.
10:15
I wouldn't expect it to be there,
and so I wouldn't look for it.
10:18
>> Yeah, yeah so you make the point about,
you don't have to be an expert.
10:23
And that helps reinforce the idea to
spread the responsibility around the team.
10:27
Everyone has something to contribute,
right?
10:32
>> And I wanna clarify.
10:35
>> [LAUGH]
>> I don't believe in experts.
10:37
I think there's beginners.
10:39
I think there's people who
are an intermediate level.
10:41
I even think there's
experienced programmers but
10:45
even when I've looked at experienced
programmers coming to a new topic,
10:48
the best I've seen don't
approach it arrogantly,
10:52
they're in there trying to learn as much
as possible from a beginner's stand point.
10:57
And to say that you're an expert,
that you determine what
11:04
code passes and what code doesn't
pass is to miss a lot of good ideas.
11:09
The code determines that.
11:14
Our Results determine that.
11:15
>> So being more open minded and flexible,
11:17
those are all real positive things
in a team situation of course.
11:20
We also touched upon, and you made
the point, that code review is not
11:25
specifically about finding mistakes,
so sharing knowledge and information.
11:28
And then the third thing, again,
11:32
was I think an important point to make is
that it's easy to think of code review as
11:34
this other thing that you have to do and
it's distracting from writing code.
11:38
So, you make the point that code review,
11:43
it's a mistake to think of it as
not part of the coding process.
11:45
>> Yeah, so
those two are very connected together.
11:49
Doing code review Is like being
able to jump into a project
11:55
that's being finished and
understanding, and
12:01
being able to interpret how it's working
without actually having to build it.
12:06
So whereas your fellow student might be
building each project from scratch and
12:11
trying to like divine some great way
of programming over and over again.
12:19
You're actually spending 30 minutes each
12:26
time going through the entire
project over and over again.
12:29
And as long as you've done the project
before, as long as you've attempted it,
12:34
then you get to see all these
different Approaches and
12:38
ways to take on the problem.
12:40
And that inevitably gets absorbed into
how you think about the problem in a way
12:42
that's much faster than trying to build it
over and over and over again from scratch.
12:47
I think that's a lot of where innovation
comes from in the tech industry
12:52
is that we are more open to other
people collaborating with us and
12:57
more open to accepting
good ideas from elsewhere.
13:02
>> Yeah, and I really enjoyed the point,
to, of the reminder that becoming
13:06
a better developer which is something
that obviously we're all typically
13:11
tuned into or wanting to do that
writing code is an aspect of that but
13:16
reading code is something that we do or
should be doing more often.
13:21
>> I remember reading that
early on in my development.
13:25
You have to read source code.
13:28
You have to read source code.
13:30
There's only so far you can take
your skills and abilities without
13:32
having that moment to go,
this is how I've been doing it.
13:39
How does it weigh up against
someone else's way of doing it?
13:45
And what are they doing well?
13:48
What am I doing well and
how can we merge these two ideas?
13:50
That's innovation.
13:53
>> Yeah, absolutely.
13:54
So in terms of actions,
13:56
to be successful at code review,
first of all get everyone involved.
13:58
>> Yeah.
>> It should be part that's equally shared
14:02
across the team integrated
into your process.
14:04
>> Yes.
>> The more that you can make it just
14:07
part of what you do the more
likely you are to keep up with it.
14:09
And actually stick with it.
14:13
>> And make it easy and
smooth for yourself.
14:14
Like when we started doing these
code reviews for students,
14:18
we didn't have automatic validation.
14:22
We didn't have automatic downloads.
14:24
All that had to be built.
14:27
And the amount of time it saved me
the amount of frustration it saved me
14:29
over the past year has been extreme.
14:35
>> Right, yeah and I guess related to
that is like instead of checking syntax,
14:38
put a renter as part of
your build process, so
14:43
that's not where you're
spending your time.
14:47
Let a tool take care of those repetitive
things, that tools are really great at.
14:49
>> Exactly and if you're working on the
same project you could integrate a series
14:52
of just basic tests just so you know
that you're meeting those user stories.
14:58
But what we're concerned about is ow
we're architecting the entire code.
15:03
What we're actually doing to organize
it so it's easy to read, and
15:09
so it's efficient to function.
15:14
>> Excellent,
have you ever used a checklist?
15:16
Do you find checklist
helpful to remind you like,
15:19
hey I'm about to do a review these
are the things I need to do?
15:22
>> I hate to say it, James.
15:26
>> [LAUGH]
>> I'm not naturally an organized person.
15:28
We do use user stories,
I use user stories all the time and
15:32
I have things that I'm constantly looking
for in my own mind, but oftentimes for me,
15:36
revealing students, I have to change that
based upon which student I'm reviewing,
15:42
because I know where they're at,
and I try to follow that.
15:48
>> I see so
you already are thinking about,
15:53
like I know this person tends to
make these kinds of mistakes or
15:55
needs this kind of support;
>> I try to keep that in
15:58
mind as much as possible.
16:01
I think it's a good idea to keep
involved Where your team is at relative
16:03
to yourself and try to introduce things
that you think the team can latch on to.
16:08
So we can step up to a higher level.
16:15
>> Or support that person-
>> Exactly.
16:17
>> In their growth and learning.
16:18
>> Yup.
16:20
>> Excellent.
16:21
Well, thanks for joining me on this
show on talking about code reviews.
16:21
This has been great.
16:24
>> I hope it was useful for you.
16:25
I hope there's more people that get more
from their learning by doing code review.
16:26
I think it's an excellent way to go about
16:31
building your skills as a new developer
and as an experienced developer.
16:35
>> For additional information about
code reviews, be sure to check
16:39
the notes that accompany this video for
links to additional resources.
16:42
Also rate this video let us know how we
are doing or if you have a topic that you
16:45
would like to see us discuss in the future
let us know about that too thanks for
16:49
watching and we'll see you next time.
16:53
You need to sign up for Treehouse in order to download course files.
Sign up