"Design Thinking" was retired on May 1, 2021. You are now viewing the recommended replacement.
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
Well done!
You have completed Dev Team Show!
You have completed Dev Team Show!
Preview
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
Related Discussions
Have questions about this video? Start a discussion with the community and Treehouse staff.
Sign upRelated Discussions
Have questions about this video? Start a discussion with the community and Treehouse staff.
Sign up
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 upYou need to sign up for Treehouse in order to set up Workspace
Sign up