Making the Most Out of Code Reviews by Mariatta Wijaya

Speaker 1: Come on, no.

Speaker 2: Hi everyone, I'm really happy to be here. This is my first time in Django Kong and my first time in Philadelphia. Yeah, so just a bit more about me. I guess I work in Vancouver, Canada. I work at software engineer for Sony Pictures. And I'm also a mentor for Ladies Learning Code. It's a nonprofit organization. We have beginner-friendly workshops for women and children to learn how to code. So then I'm talking about code review and how to make the most out of it. So let's think a little bit about this place. Code review Two simple phrases, almost like a buzzword.

Speaker 2: Programmers write code. People will view it. Write comments To me, code review, especially when I just add it, it's extremely daunting It's like everyone is so eager to find out something. Every line, every lateral in the empty space is scrutinized. People would tell me that what I was doing is wrong. It's my best practice. I didn't always know what the best practices are It would tell me I need to reorder my input statements. Should be grouped

Speaker 2: certainly. Alphabetical order. One time I was told my implementation was completely wrong. The whole thing needs to be rewritten I felt like such a failure. I made so many mistakes. Maybe I'm not good at it. Maybe I shouldn't be good at it. There was one time I was told I should align my stars So I did. I aligned my stars in the most literal sense. My stars in my JavaScript file

Speaker 2: I made them line up perfectly. But I know I was wrong. I'm sorry I did this mistake. But the step worth readapting a problem. request for this. Spending a few extra minutes, create a different pull request, fill it a few spaces, asking people to review this. I'm not sure this is really the point of doing code review. Very often during reviews. People are arguing, developers thinking that their own way is the best way, nobody wanna back down, passing the ways

Speaker 2: We also have team members who didn't prioritize to interviews, or rather working on their own code, finishing their own task As a result , codes are on time. Wouldn't release the features we wanted in deadline. Everyone's disappointed. Code review can really be a negative environment Gotta start asking, is this worth doing? We know it's as practice, but it's also source of your daily stress

Speaker 2: Cost delays. Maybe we don't need it. Maybe you could just trust our developers. They know how to write good code. Shouldn't they be writing the mass code all the time? So we don't need this, maybe. I don't know Before we can answer whether this is worth doing or not, I think it's really important to know why why we even do it in the first place. Code reviews is not just about readability. It's not just about following standards, conventions. It's also not the place to evaluate someone's skill.

Speaker 2: It's not the place to determine who's the good programmer or who's the not as good programmer. You might come with assumption that senior developers do not need their code to be reviewed So you only scrutinize the new cameras code? That's not the point. Think of how negative it can be to be a new multi-member The real goal. Why we do cooperative? Because we care about quality. Next question. So what's quality

Speaker 2: code? I know some of us will start thinking about what is that? Pat 8 But there's more to it, there's more things that determine quality than just standards, conventions. You want us to make all those code smells. Things like magic numbers, duplicated code. Overly complicated functions. There are so many things to look up for. Does it have unit test? Is there enough code coverage? Documentations also important, you want to look

Speaker 2: look for that. Care about performance If the algorithm can be optimized maybe security considerations There's just so many things that determine the quality of the code from being fixated on So eight or coding standards. The real goal here is that we don't want to introduce technical depth. We want to make sure it's easy to maintain. And that's why we are willing to spend this extra time, extra effort doing the reviews

Speaker 2: for quality. So now we know it's needed. We know the goals of reviewing code. So now you're the Blue Incode and you see something that should be fixed. You see something that could be improved. So speak up. Tell the developer that I think that's a better way to do things And help them. Help them be better in coding. Oh, sorry. Oh Does this work? Okay, so just start by providing code samples, show them how to refactor the code

Speaker 2: maybe And you may want to offer how to do pair programming with them if it's quite complicated. Maybe they're new at this and they didn't really know much. Provide learning resources. Um maybe you went to PyCon, you went to DjangoCon, you learn about a lot of new things. And your coworker didn't We should share the knowledge so that they could learn and their next code could be better. Part of doing code review is for the purpose of learning and knowledge transfer. I think some of us might have missed this goal.

Speaker 2: I I know I did. I thought that in the beginning I'm not qualified to do code reviews. I didn't know how to comment But the reality is by participating in reviews, participating in discussion, asking questions, reading lots of new code. I get to learn so much. I really become a better programmer through Gold Review. If there is ever any confusion, code review is the time to ask questions when programmer just finished writing it. They know how to answer your questions. Don't wait

Speaker 2: six months to ask what is this code about. This is known as increasing your buzz factor. You always want to make sure that there are other people in the team that knows about your code. So really code review is important, it's essential if you care about quality, care about knowledge transfer. You wanna wanna your team member to be more experienced? Definitely have code review process because learning is so valuable. I just wish that we don't be fixated on the term review. It's really not just about

Speaker 2: scrutinizing, commenting, pointing out mistakes. It's really more than that. Think of this process more like a conversation among friends, among colleagues. It's not about judging So we know we need this. We know we need to have this kind of conversation. We want to learn. We also know it could be negative. How do we make this positive? How do we keep how do we make this great again? Start with having some empathy I know that some of us may be okay being told you did something wrong, you need to fix things, but

Speaker 2: not everyone can handle being scrutinized all the time, every day. Having empathetic comments will those comments will be well received by everyone. Before jumping to read reviews, read codes. At the very least, try to understand the context. What is this code doing? Read the title at least to avoid any misunderstanding. As programmer do try, make the effort to create smaller merge requests. Make it easier for people to review your code. And oftentimes when when we review we are really quick to point

Speaker 2: out mistakes, to criticize. But when the code was good to begin with, nothing is fixing You even learn a thing or two. Often we say very little. We say nothing. We say plus one. You had the federal list. Say thank you. Don't take this for granted. I know it's it's the programmer 's job to write code, but you could still show appreciation So a lot of projects probably have a contribution guideline. I think you should also have a code review guideline. Letting people know what's expected out of this process.

Speaker 2: So what what should go into the guideline Again, coding standard. You should have that. It's not enough as a guideline. It's a good start. Apple code of conduct. We are bound to excellent code of conduct here at a conference. It shouldn't stop when you go back to your company, to your private organization. You should impose that in your own company. Think about what to do when there's conflict among developers. Each team might handle this differently. Specify a timeline.

Speaker 2: You may want to impose that the review gets done within a day or two, but no longer than that. Also good idea to have a list of backup or alternate reviewers. Like don't limit reviewers to be just the same people all the time, include outsiders. They could offer you extra additional perspective. So I think we know a little bit more about this whole process of reviewing. Let's let's do a little bit of practice So this is an actual uh code that I found in my legacy our legacy code and um I'm maintaining it but There's a two

Speaker 2: simple lines. It's a OS. system , RM dash, Rf, some file name. By the way, we notice it's not it's not uh following standards. Perhaps there's a better way to format strings, right? This is the old way we should use format. Should I change it? So now it's good. Piling is happy. But I I still don't understand what these do I did some, I asked some questions. What is OS. system? It's a way to invoke command line script from Python.

Speaker 2: So while searching, I also found out that I was supposed to be using a different library. This is not desired. Great, I learned I learned new things So I change it. So I now we know what it's doing. It's executing RM dash Rf We're trying to delete the file and if there's any problem with it, we don't care. I don't know I'm not I'm not comfortable with this. What if somebody forgot to pass in the file name? There should be a better way to delete files from Python

Speaker 2: You could do OS dot remove. Same some package. Why why resort to executing Command line script. If there is a problem, I wanna know. I wanna do a chai catch exception from this So it looks it looks better now. I'm much more comfortable with this than the previous code. So Pass. Let's ship this, ship this now. Thank you.

Speaker 3: I think we have time for one or two questions if anyone wants to step up and ask a question.

Speaker 4: Hi, thanks for doing the talk. Um I have a question. Do you when you're doing code reviews, do you work from like a style guide? I mean do you just use like say PEP 8 standards or do you develop internal style guides for coding? Because that's something like my organization is definitely grappling with right now.

Speaker 2: Yeah it's really different. Um we try to use PEP 8 like I've used PyLint and I also like use the title that comes with PyCharm, but every team members have their own preference. Like I have team that prefers to use underscore and I have teams that prefer to use like camel casing so everyone has their own preference. It's something to be discussed with your own team Did I answer your question or

Speaker 4: sort of. I I was really more curious, do do you like do you try to come up with a style guide internally when you have that conversation with your team? Is there like do coding style guides work, I guess

Speaker 2: It works if people are made aware of it. Like with work in a really large organization, sometimes knowledge got lost. We have something written about this and if people are not aware of it, they don't read it and we ended up having to correct people to during reviews, telling them, hey, look, we have this guideline. Go follow it.

Speaker 5: Um I have tried to Offload some of the code review work that I need to do for my team to kind of the entire team by kind of requiring them to get approvals on their merge requests. Um but I find that a lot of times I don't I'm not I'm not exact and I need to ask and I'm not exactly sure what it is, but they The developers on my team don't often provide a lot of feedback to each other. I just see a lot of, yep, approve, approve, sometimes find some things. But do you have any tips for like kind of I mean this code conversation idea I think is gonna help, but just tips for like encouraging developers to give each other um constructive feedback and kind of establishing that culture of

Speaker 5: Look, just because you're giving someone feedback, you're not attacking them like this is this is an opportunity. Yeah. Just kind of tips on.

Speaker 2: I think I for myself it starts from me. Like I I know in the beginning that's all I do. I say, okay, approve plus one, right? But throughout time I realize that those feedback are important. So when I do review I do spend extra time trying to help. Like can is I know this work. Is there better ways? Let's do a little bit of more research and but the thing is not every team might have that luxury of extra time. So it's it's something to be c

Speaker 5: Do you have a tip on getting people to just even like have time in their day where they do it? Because a lot of times they just want to work on their own stuff. I don't want to spend time reviewing other people's stuff.

Speaker 2: Yeah, I I impose this for myself. Like do it at least three times a day. First thing in the morning, before lunch, or after lunch, before I leave. Just make sure something's get done. realize that you need their help they will also need your help, right? And when you set good examples people start realizing that this is how it should be, you know

Speaker 3: Do we have one more quick question?

Speaker 2: No.

Speaker 6: I was wondering if you had any thoughts on how to scale this way, way down to a very small team. Like say three developers.

Speaker 2: Yeah, when you have three developers and you're you're gonna be pushing a lot of code all the time. And um Yeah, I don't know. We struggle with this too. I also have a small team and it's just the same people again and again. Really try to get external reviewers if you you can like different team within the same company or something like that. Just so just so they're not overloaded. And if anybody they've been assigned too many reviews, they should feel free to say I'm sorry I cannot do the review today. Maybe you want to ask somebody else to do it. So

Speaker 3: Awesome. Thank you so much, Miriata. Thank you. He is