Welcome to the Dev Team Show my name is James.

In this episode, we're gonna talk about code reviews.

Joining me is Nicholas Hampton.

Nicholas is a member of Treehouse's technical success team.

Welcome to the show, Nicholas.

>> Thanks, James.

>> So I wanted to start just by kinda giving a basic definition of what a code

review is just to make sure that we're all like on the same page.

>> Of course.

>> So from my perspective,

a code review has a component of reading code, of course.

That's sort of the, one of the review parts.

But you also need to like, run the code typically too, right.

So you're executing whatever it is that the code is written-

>> Yes, you've gotta unpack the project

deploy it and try a trial run of the program,

of course check for any errors that might occur.

>> So there's a validation part in there too.

So you're validating the behavior of the application and making sure,

does this thing do what it's supposed to be doing?

>> Exactly, and making sure that it satisfies some user stories.

>> Right, and then you take whatever you've noticed,

whether it be positive or negative, and you're reporting that back to the user.

And then how you do that,

there's a variety of different ways that that's done too.

>> Yeah, but for the most part, you wanna keep in mind that

this is somebody working on code, that's just trying to improve it.

So you want as much as you want to point out anything that goes wrong,

you also want to point out anything that they're doing right.

>> Right, positive and negative comments, not just criticism all the time.

>> Exactly.

>> Which is easy to fall into that habit.

So in terms of the value of code reviews, if you search the web, you'll find

opinion after opinion that people are on board with the idea of code reviews.

I don't think that that's, an idea that's challenged very often,

but I'm gonna read a Steve McConnell quote.

>> Yeah. Because I think that he kinda

gets to the point of some research that he did,

some studies or that he's paid attention to that really drives the point home.

So Steve McConnell from his book, Code Complete, which is an awesome book says,

software testing alone has limited effectiveness.

The average defect detection rate is only 25% for unit testing,

35% functional testing and 45% for integration testing.

And in contrast, the average effectiveness of designing code inspections are 55 and

60%.

So substantially higher than like other forms of testing that you can do

>> Those are amazing numbers and

I completely agree with those results.

But I think the focus of those numbers are misleading and in the wrong place.

>> Interesting, so I've got a confession.

I'll just get it, I'm getting into the habit of doing this on the show.

>> [LAUGH] >> Get it

out of the way right off the bat.

I've probably done about everything wrong that you can do about code reviews.

>> Father, forgive me.

>> [LAUGH] So for instance, I have found myself in

various situations being I think I've heard you call it the GateKeeper.

>> Yes.

>> So what code reviews meant in that situation was that everyone who committed

code to the project and made changes, I was reviewing that.

Furthermore it wasn't really integrated into our process.

So I kinda would have to review the commit history.

>> Yeah. >> [LAUGH]

>> The project, and

be like right, I left off here on Tuesday, it is now Thursday,

I'd better read some of these commits, some of these check ins.

So I would start to do that and

of course the tool didn't support providing feedback, so I'd be writing

things down in an email typically, which is disjointed from the code.

>> A mess.

>> A complete mess and over time >> And it wasn't like we weren't getting

any value, but gradually over time because nothing really worked well and

it was always one direction, like me giving feedback to members of the team,

it breaks down and we would quit doing it, so whatever value we were getting out of

it, we stopped getting it because we just quit.

>> Yeah, and I think a lot of that is being able to address

code review as you would just any part of the development workflow.

You have to create a workflow for yourself.

And GitHub does that really well in terms of you could that daily peer reviews

based upon pool requests and just going over that even as a group.

And going through and approving each pool request.

On then, at the end of the day.

Maybe having someone take a quick look over at, over all the poll request that

have been made just a quick look at the reviews that were done on that.

>> Right, so real quickly poll requests,

which I think that's terminology that others are starting to adopt.

But originally, what I think was a GitHub idea, right?

>> Yes.

So get is the source control that they're using, but

they're layering on this idea of a PR or poll requests.

>> Yes.

>> And so you're working on a branch that's separate from whatever your main

and master branch is.

>> Yes.

>> You make all your changes and

then you say hey I'm ready to merge this upstream or into main branch.

You do a PR which basically is sort of a fancy way of saying hey,

can you review this thing?

At the point and time that you say, I'm ready for

a review, Nicholas can you review this for me?

>> Yes, and a lot of times, the beginning step of that whole

process is to fork the repo that you're working on in the first place, so

forking the review and making a copy of it for yourself.

Making changes to your copy and

then making a poll request to ask whoever's overseeing this project.

Hey, I've made these changes for these reasons, could you look it over?

And commit it if it's good, or make comments if it needs improvement.

>> Right. >> And it's a solid way of introducing

a workflow for your code review.

But also another problem is, I would get annoyed if I had to do all the.

>> [LAUGH] >> All of the pool requests for

a particular project.

Or if I was reviewing all of those pool requests.

>> All right so let's dig into that a little bit because,

like I just mentioned a minute ago,

that's something I've absolutely been guilty of is being that gatekeeper person.

But in fact, to make code reviews as effective as possible,

it should be something that's equally shared across the entire team.

Yes, and I think guilty might be a key word there, right?

>> [LAUGH] I wear guilt well.

>> [LAUGH] But you're hogging all this learning material.

Everyone in your team is studying things.

>> Yep. >> Some of them are going to overlap and

some of them are going to be way out on a left field.

>> Right. >> And so everyone has this

pool of knowledge to themselves.

But, when you designate one code reviewer,

that one code reviewer gets little pieces of all that knowledge.

>> Right.

>> And then no one else dies.

>> It stops there.

>> So every team member is isolated from every other team member's poll of notch

if you were to split that up where peers are reviewing peers,

then like a pool of information just puddling on itself,

we get information from everyone else.

And actually I can in that situation slightly slow down

the amount that I'm taking in from outside sources.

Because I'm getting all that from the sources inside my own team.

I'm seeing all that play out in a code base that I'm currently working on

instead of trying to decipher someone's example that's out of context.

>> Yeah, exactly, which is great.

And it also it helps just like start those conversations about the code you know and

people say, hey I noticed that you did this thing over here.

Can you explain that to me and you can use that as a starting point and

jumping off point to have some of those deeper conversations.

>> Exactly, or even deeper than that.

I notice you're throwing this exception error.

I don't really understand what the wording is.

Is there a better way to communicate to our team and

the user what this actually means.

Is this in the right format that we wanna look at?

>> Right.

>> Even inside their own project.

>> Now, you wrote a blog post recently about some of the current

misunderstandings or misconceptions around code reviews.

And one of them that I thought was really interesting was,

you talked about this expert like mindset.

So for instance over time you develop a way or a habit of looking at things and

saying, I recognize this problem and this is the solution that I do.

But you might miss an opportunity to think about it differently.

And that's where if you have your entire team involved in the code

review you made the point that someone, maybe even with less experience,

they're actually going to have a better chance at seeing something differently and

asking the question that says well, have we considered this?

Did we think about that?

>> Yeah, and I think part of that is someone who's fresh and

new to the table, often times is often just thinking

a lot of detail about something that we've taken for gratitude for a long time.

>> right. >> We learned that two years ago,

I don't have to reassess that.

I'm just gonna glance over that.

That's just a meta tag.

>> Right, you can kinda short circuit the thought process you originally went

through maybe multiple times.

To establish that pattern or that habit and

you just sort of just cut to the chase.

And say, hey, I know how to do this.

>> Example of that, I was trying to do a code review for

one of our students and I kept on getting the same error.

And I was like, nothing is wrong with this chunk of code, then I spent like a good 20

minutes on this chunk of code in for I looked directly above that piece of code,

and they had actually written the same piece of code

on a different configuration that was overriding their lower piece of code,

because it was then a document on ready.

So, this was running first, then that, and this didn't matter all.

>> Right, right.

>> It's something I would've never thought about, because I wouldn't think that you

would have to write those two pieces of code twice you don't.

But fresh eyes will catch that pretty quick.

I wouldn't expect it to be there, and so I wouldn't look for it.

>> Yeah, yeah so you make the point about, you don't have to be an expert.

And that helps reinforce the idea to spread the responsibility around the team.

Everyone has something to contribute, right?

>> And I wanna clarify.

>> [LAUGH] >> I don't believe in experts.

I think there's beginners.

I think there's people who are an intermediate level.

I even think there's experienced programmers but

even when I've looked at experienced programmers coming to a new topic,

the best I've seen don't approach it arrogantly,

they're in there trying to learn as much as possible from a beginner's stand point.

And to say that you're an expert, that you determine what

code passes and what code doesn't pass is to miss a lot of good ideas.

The code determines that.

Our Results determine that.

>> So being more open minded and flexible,

those are all real positive things in a team situation of course.

We also touched upon, and you made the point, that code review is not

specifically about finding mistakes, so sharing knowledge and information.

And then the third thing, again,

was I think an important point to make is that it's easy to think of code review as

this other thing that you have to do and it's distracting from writing code.

So, you make the point that code review,

it's a mistake to think of it as not part of the coding process.

>> Yeah, so those two are very connected together.

Doing code review Is like being able to jump into a project

that's being finished and understanding, and

being able to interpret how it's working without actually having to build it.

So whereas your fellow student might be building each project from scratch and

trying to like divine some great way of programming over and over again.

You're actually spending 30 minutes each

time going through the entire project over and over again.

And as long as you've done the project before, as long as you've attempted it,

then you get to see all these different Approaches and

ways to take on the problem.

And that inevitably gets absorbed into how you think about the problem in a way

that's much faster than trying to build it over and over and over again from scratch.

I think that's a lot of where innovation comes from in the tech industry

is that we are more open to other people collaborating with us and

more open to accepting good ideas from elsewhere.

>> Yeah, and I really enjoyed the point, to, of the reminder that becoming

a better developer which is something that obviously we're all typically

tuned into or wanting to do that writing code is an aspect of that but

reading code is something that we do or should be doing more often.

>> I remember reading that early on in my development.

You have to read source code.

You have to read source code.

There's only so far you can take your skills and abilities without

having that moment to go, this is how I've been doing it.

How does it weigh up against someone else's way of doing it?

And what are they doing well?

What am I doing well and how can we merge these two ideas?

That's innovation.

>> Yeah, absolutely.

So in terms of actions,

to be successful at code review, first of all get everyone involved.

>> Yeah. >> It should be part that's equally shared

across the team integrated into your process.

>> Yes. >> The more that you can make it just

part of what you do the more likely you are to keep up with it.

And actually stick with it.

>> And make it easy and smooth for yourself.

Like when we started doing these code reviews for students,

we didn't have automatic validation.

We didn't have automatic downloads.

All that had to be built.

And the amount of time it saved me the amount of frustration it saved me

over the past year has been extreme.

>> Right, yeah and I guess related to that is like instead of checking syntax,

put a renter as part of your build process, so

that's not where you're spending your time.

Let a tool take care of those repetitive things, that tools are really great at.

>> Exactly and if you're working on the same project you could integrate a series

of just basic tests just so you know that you're meeting those user stories.

But what we're concerned about is ow we're architecting the entire code.

What we're actually doing to organize it so it's easy to read, and

so it's efficient to function.

>> Excellent, have you ever used a checklist?

Do you find checklist helpful to remind you like,

hey I'm about to do a review these are the things I need to do?

>> I hate to say it, James.

>> [LAUGH] >> I'm not naturally an organized person.

We do use user stories, I use user stories all the time and

I have things that I'm constantly looking for in my own mind, but oftentimes for me,

revealing students, I have to change that based upon which student I'm reviewing,

because I know where they're at, and I try to follow that.

>> I see so you already are thinking about,

like I know this person tends to make these kinds of mistakes or

needs this kind of support; >> I try to keep that in

mind as much as possible.

I think it's a good idea to keep involved Where your team is at relative

to yourself and try to introduce things that you think the team can latch on to.

So we can step up to a higher level.

>> Or support that person- >> Exactly.

>> In their growth and learning.

>> Yup.

>> Excellent.

Well, thanks for joining me on this show on talking about code reviews.

This has been great.

>> I hope it was useful for you.

I hope there's more people that get more from their learning by doing code review.

I think it's an excellent way to go about

building your skills as a new developer and as an experienced developer.

>> For additional information about code reviews, be sure to check

the notes that accompany this video for links to additional resources.

Also rate this video let us know how we are doing or if you have a topic that you

would like to see us discuss in the future let us know about that too thanks for

watching and we'll see you next time.