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