Fog Creek

Code Reviews in Kiln – Webinar

 


This covers the who, what, when and why of code reviews, before diving into how code reviews work in Kiln itself.

Transcript

Let’s talk about code reviews in Kiln On-Demand. We’re going to go over what are code reviews, why you should use them, when you should them. What to look for during code reviews. How to create a code review, commenting and replying on a code review, working with an existing review, and all of that is going to be touching a basic code review work flow.

Code reviews are a way to facilitate collaboration about your code, that’s really what code reviews are. By reviewing code it allows you to do a few things which I’ve listed here. You could improve your software through feedback and mentoring, those are really valuable things that you can have there. As well as help you ship nearly bug-free software.

When’s the right time to review code? Really, any time. One way to think about it is to review code when that code is destined for somewhere else, that’s going to be used by something else. That sounds really vague, right? Specifically, things like other developers and repositories. Other systems, like build and test systems. Most importantly, your customers. Reviewing code from when you’re deploying from one environment to the next, or from one repository to the next. Really any time, the goal for you is to get that objective opinion in there, and that’s what you get by having your code reviewed.

We recommend that code reviews be performed by everyone and anyone. When you’re performing a code review, you’re going to be evaluating it for two things. How does this code match up against your team’s checklist, and how does the code fit within your organization’s guidelines. Those are two big topics that I’m going to go over. Specifically, what is in those checklists and those guidelines is up to you, and your team, and your company.

Let’s talk about guidelines. When I say guidelines, I like to think of these with the question, “What do I push to Kiln?” First off, don’t mix functional and nonfunctional changes. Secondly, keep your change sets small and on topic. Try to get that commit message to tell a story, so that when somebody comes back to that message they can get a glimpse into what you were thinking and how you were approaching what you were doing. Third, be cautious of making large changes, especially in common files. The reason for that is it can sometimes cause time consuming merge conflicts, especially across branches.

Now for checklists. A checklist is what you’re going to use when you’re reviewing the code as a reviewer. Things like, does the code work? Evaluating the code for security risks. Does that code conform to your company’s coding style guidelines? Does the code structure or design fit? Let’s take a look at how to create a code review. If you haven’t seen Kiln before, the first page you’re going to see when you log on is the activity page. That’s what I have here. You’re also going to be able to navigate around. You’re going to be able to view things like the repositories at the top, or the reviews.

The activity page is really, really helpful because it shows you all sorts of activity involving your repositories. The code that was pushed, repositories created and deleted, code reviews approved, among many other activities. The repositories page is actually pretty nice and full-featured as well. It allows you to manage your repositories. You can create projects, you can group them, set permissions, among a few other things. The reviews menu, in essence, allows you to see what reviews you’re interested in.

To create a code review, we have code pushed to Kiln. Today we’re going to do a bit of role play. I’ll be acting as two developers, Erin and Jamie. They’re working on creating a proof of concept for a web application, and they’re working for a company called Whisk which offers a car sharing service. I’ve gone ahead and pushed something up for us as the user Erin, as you can see here on the activity page. It just said Erin pushed to the repository on the master branch with a very vague commit message from a few minutes ago.

Now we’re going to create a review. To do that, hover over the change set like I have, and you’re going to see a review button. Click the review, change the title maybe to something more appropriate. I’m acting as Erin right now and I’m opening this review because I need an objective opinion about my code. I’m changing the title to reflect that. I’m also going to choose Jamie as the reviewer, and we’re going to give Jamie a quick description. Here’s the POC in Ember JS, easy enough, and then click start review.

Review has been created. You can see the code review number at the top here with a link. We’ve created our review and we’ll wait for Jamie to get back to us. Now I’m going to switch actors here to Jamie, and you can tell by this avatar on the top right, as well as the chrome color window. The first thing you’re going to see, is that this review’s drop down looks different, right? This has the number one in it, because there’s a new code review waiting for Jamie’s action. He’s going click down, find the one on the code review, click it, easy enough. Jamie will also get an email as well. If you haven’t seen code reviews in Kiln before, I’m going to try to cover that here as well.

What we’re looking at right now is the code review broken up into some major components. The first is going to be the main area of the code review which shows you a diff of the code compared to the parent change set. In this case we updated the repository, so everything’s going to be new. Your major code review action buttons are on the top right, approve, needs work, reject. I’ll go over those in more detail today. Your top left will show you who the active reviewers are, which repositories effected, as well as the branch of repository, and if the repository is Git or Mercurial.

The other thing that’s important, I mentioned emails earlier, the notifications. I set Kiln to the default to subscribe to code reviews that I get assigned or create, and in doing so, I get an email notification about every event on this review. You can change it to muted or occasional, whatever you have a preference for. I can manage my reviewers on the left here, I can add more than one, I can remove Jamie. On the left is going to be my comments and any replies we have today. The main, the center top here has two views for you. Right now we’re showing the by change set view, which means that you can have more than one change set on a review. Really helpful when you’re adding changes that are all related.

The by file review is also helpful too, because the items in bold text show you files that you have not yet seen. If you’re looking for something in particular, like today we’re going to be focusing on the index.html file. I’m Jamie, and I’m looking at this code review, and I’m going over the check list that my company and team have put together. The check list says no hard coded values. I can immediately see here in these list items that we have some hard coded values. I also happen to know Ember JS. I know that these should be fixtures instead, so what I’m going to do here is click and drag. What that does is it groups these lines of code together for me. Click attach lines to comment, and I can just tell Erin that that just needs to be changed.

I think moving these two fixtures is the way to go, and we can add an emoji in there, so that way I can show that I’m a human on this side. At this point, this is the first step of the basic code review work flow, and this code isn’t quite ready yet to be consumed by anything else, so I’m going to reject the review. Jamie’s job is done, I’m going to switch back to Erin’s view. You can tell by the avatar here. Kiln’s going to automatically show us that we have a new unread code review or comment. You can see that by the yellow pop up.

Let’s take a look as Erin, we’re going to look at the code review and see what Jamie said. Okay so you said it’s looking good, wants something moved to fixtures, and we rejected the review. Erin wants the review approved, so let’s click this blue rectangle and see which lines of code Jamie wants us to focus on. Great, yeah, so I can see that. What I’m going to do is reply back to Jamie. I like to keep communication at a high level with code reviews just to keep people sort of engaged and active. Erin’s going to go ahead and make those changes, and then we’re going to push those new changes to Kiln.

What we’re going to do now is I’m going to copy the code review number so I can include that in the commit message of my push. That’s going to do something really neat for us. It’s going to automatically add this change set to the code review so we don’t have to do it. Saves us a lot of time. All we have to do is copy and paste something real quick. I got my commit ready here, I’m just going to finish the message for you. Review and then K and then the number. This is the key part of this commit message to save yourselves time later on. We’re just going to add that and then push it out.

Great, so that’s pushed, and our change set should automatically show up on the review in a moment. There is goes. You can see that the review’s drop down changed again, which tell you there’s something new. Really useful. You can see in the somewhat fine print at the bottom that Erin just now added a new change set, and the reviewer’s status has been reset. It’s as if it’s a new review. Erin’s looking at this review and she wants to make sure that her new changes are on there, so she’s going to by change set view, and she can see her second change set is here.

Now at this point the review’s been reopened and the ball’s in Jamie’s court so to speak. We’re going to switch over to Jamie, again, the green avatar. Jamie’s going to look here and see what other change sets were added. We can see that there’s a new one because that has the bold text. We’re going to click on that, and we can see what the changes were. Great, so that’s starting to look good. What we need to do now is just go back to our checklist here and see what else we needed to review this code for. I’m going to go to by file view, and I want to make sure I’m looking at the index.html. Aha, so I can see something pretty easy here. There’s no style sheet. We’re going to an Ember JS app. We should have a style sheet of some kind.

Again, we’re going to click and drag on those lines that we want to focus on. Click attach lines to comment, and then we’re just going to say the app works. Real quick, add comment, easy enough. At this point we could use the needs work button, but it doesn’t prevent code from moving forward to something else. We’re going to use that here, and again I’m going to switch back to Erin’s view. She’s going to get an update in the review’s drop down. We can see on the left here that Jamie added some comments, so we can click the blue rectangle and find out specifically what he’s referring to.

Great, so we need a style sheet, that’s not a problem. I happen to have that ready to go, and I’ve already pushed it up to Kiln. What you’re not seeing here is the code review hasn’t been updated with that change set. Let’s go look at our activity page and find out why. We can see here that Erin pushed the CSS and background stuff to the Whisk repository a few minutes ago. What’s different here is that there’s no icon over here. It means that this change set doesn’t belong to a review, so we have to add it. We’ll click review, and instead of opening a new review, we’re just going to click add to existing review.

Choose the existing review we have opened, and now you can see that, in addition to the two existing change sets, we’re adding a third. We’re going to click add to review. I’m also going to just let Jamie know that I’ve added it in there. Switching back to Jamie’s view we can see Erin’s comment. We can see the change set’s been added. We can check the by change set view and see the new one. Focus on what those changes are specifically. Okay, great. I’m a CSS wizard here and everything looks fantastic. At this point, the review’s read to be approved. I’m just going to let Erin know, looks great. Then we’re going to click approve.

There you have it, we just went through one possible code review work flow. We also covered what are code reviews, why and when to use them. What to look for during a code review, as well as commenting, replying, rejecting, using needs work, adding to existing review, and approving a review.