 Okay, so welcome here. And I've heard this is one of the last sessions. So thank you for staying. My name is Emilia Maki. I work for Red Hat. I've been working on OpenStack for five years, and I've been reviewing code for almost four years. So I have some experience, and I would like to share it today. Before I start, who is already doing code reviewing in OpenStack Garrett? Okay, so my wish is that nobody says, yes, so I can teach you something. So today, this is a hands-on live demo. I don't have slides and I don't have any, I didn't prepare anything. Like I'm going live to a patch from a coworker, and I'm going to show you how I'm doing code review. So first of all, this is my personal experience. There is no, what I mean is that this is not like, I'm not going to write a tutorial of this because this is the way I'm doing, right? Fungi here is doing code review. I'm pretty sure he's not doing like this, okay? And that's great because we don't work on the same project and we don't have the same, you know, we don't have the same experience, I guess. So everyone has their own style of doing code review, and my goal today is that you can take some takeaways and leave the room with some fouts about code review in OpenStack. So first of all, you need to do the code review. You need obviously OpenStack account. So I'm not going, I don't have time to go in details of this thing, but if you need any help on creating the OpenStack accounts, it's documented already, but if you need like help, like talking to someone, you can come to me, you can come on IRC, OpenStack in fraud channel, maybe. We have a bunch of people, very nice and responsive, they will help you, I'm sure. So let's say you have an account, you're logged in Garrett's and there is a patch. This is fixing a bug, or this is doing something that you're interested by. This is a patch from Alex, he's a co-worker. We work together and I'm going to open the link, which is already open, but I'm going to look at the patch. The first thing I'm doing is, and again, this is me, okay? I'm reading the title. So there is a title in bold here, and then there is a description. So the first thing I'm doing, if there is no description, I'm not very happy because I don't understand what the patch is doing. So I like when there is a description. So the description says something about the patch and if the description says enough things about the patch, I'm very happy because when I will read the code, my brain will make connections between what I've learned in the commit message and what I'm seeing in the code. So what I'm saying is that if you're submitting patches in OpenStack and you spend two days writing code and you're so happy, you submit the patch and you don't write a commit message, you're just failing at sharing what you're doing in code. So please write commit message. A commit message is explaining what you're doing. There are some good documents in OpenStack documentation about how to write commit message and if you want to learn more. If you look at the commit message here, we have something called depends on. So it means that this patch depends on another patch in OpenStack. So if I click on this one, I can see that there is another patch that Alex did in another project and it's required to have this one. So the first one can work correctly. So I'm not going into details, but this is basically how it works. It's a dependency. The second thing is the change ID. You don't have to generate that. It's when you use a Git review, it will generate a change ID for you. It's just a unique identifiers for the patch. And the last line is, it's interesting. It's implements blueprints and the name of the blueprint. So if I click on it, it would direct me to Launchpad and I can see the name of the blueprints. Okay, so this is the patch. There is a title or description. So I know what the code is doing and there is the blueprint link. So I can go on the blueprints and I can click on it on Launchpad and I can see what Alex is doing. So it's trying to provide a tool to capture the status and the logs from the environment. So we don't care today about what it's doing, but just to say that there is a blueprint is trying to solve a problem. And this problem is documented in the commit message. So that's very useful for the reviewer. So they know what problem you're trying to solve and if it's part of a specific discussion in upstream, usually it's done via the blueprints. Okay, so we just spent five minutes and we didn't look at the code yet, right? This is not finished. Next step I'm doing. Okay, so we have commit message. We have a reference to the blueprints. Good. Before going to the code, I'm looking at the CI jobs here. So if you look at here, we have a bunch of jobs. It's a functional testing, unit testing. It's a lot of things. So if you are not familiar with the projects that you are reviewing, like if you don't know, this is a patch in Tripolo. Tripolo is a project to deploy OpenStack. If you are not familiar with the CI in Tripolo and if you don't know what everything like this means, you need to of course get involved in the project and ask people how do they test the project like Python Tripolo clients here. This is the projects that we are patching. So you need to learn how it's tested. For example, we have a job for PEP 8. So this is the basic Python jobs for validating the Python syntax. We have a unit testing jobs, Python 3, that's the unit test. We have functional testing jobs, et cetera. So the first thing I do when I review the patch after the commit message, I look at the CI jobs and if I see all green success, before looking at the code, I already know that Alex did something that is not breaking the CI. This is not going to, I think in theory, it's not going to break all the things. If I see everything in red, I'm pretty sure something wrong in the code will break everything. So this is the first thing I'm looking at before going to the code. If I see a mix of green and red, I will look at the code, but I will consider that something can break some jobs and something can be ignored by the jobs, right? So for example, if there is a little syntax issue that is not critical, it might break the PEP 8 jobs, but maybe it won't break the functional testing jobs. So I will consider that when I read the code and I will probably find where it's failing in the logs and I will drop a comment and say, hey, the code is good, but there is a little typo here. So fix it and I will plus two. So we spent 10 minutes almost. We didn't talk about code yet, right? Commit message, all the doc about what the patch is doing, the CI jobs, what else? Okay, so, okay, let's go into the code. So I will click on the first file. So the first file is a release note. So if you are not familiar with release notes in OpenStack, we use something called Reno, or Reno, I don't know, in French I say Reno. So you can say Reno if you like. This is a Python tool which is very useful and very easy to use that will generate release note for you. So every time you submit the patch in OpenStack and when the patch is implementing a new features or fixing a bug, we want our users to know about this bug fix or this new feature. So instead of writing big release notes at the end of the cycle, we ask to the developers every time they contribute to OpenStack to write a little text about what is doing. So this is the release notes and this called Reno. So Reno is the tool you want to Google if you want to learn about it. The next thing is code, code, Python code. I'm not going to the review because we don't have time, but oh, this file is interesting, that's a unit test because I can see in the path it's a triple client slash test. So this is unit test, great. I will check that the unit test are actually testing something useful and I will also validate that the unit test coverage is good comparing to what Alex is doing in the code here. So this is the code that does something. I'm looking at the code, spending time, but right now I'm very fast. So this is great, the code is good. It's using libraries like OSC, it's something in OpenStack, so that's great. I like it. I spend one hour to review the code. I think it's okay, the commit message is good, the CI is good, we have a release note, we have unit tests, the code is, to me, good. Okay, so I will vote. So I click on reply and because I'm a core reviewer on Tripolo, I will plus two because this is the way I think we can move forward with the patch, so I will plus two. And because I'm nice, I will say, looks good to me. Thanks for this work. That's the extra mile of being nice in OpenStack. Sometimes it helps also, and that's it. So I did a code review. I didn't minus one because I didn't find anything, but like I said, if something's wrong in the commit message, if there is no test, if there is no release note, if the code is not good enough to me, I will minus one and I will probably be like, hey, the code is nice. Thank you for working on this bug. It has been staying forever, but there is no release note and we like having release notes so the users can know what we are doing. Or for example, if you miss the tests, we obviously need unit tests every time we change the code. So the patch is good, but it's missing something and I will mention it during the minus one. So I click on reply, I can minus one and I can drop the commands in the minus one, okay? So that's typically what I'm doing. And I think I'm running out of time, yes. So I don't know if the next people from this session is here already, but if we have any question about code review, I'm staying here and we can continue the discussion. If you need some help on your first code review, I can sit in the couch and we can go together. I have, I think I have 20 minutes. So thank you for your presence and I hope you had a good week here. Thanks.