 My name is Vaidahee and just to get a sense of the room, how many of you grew up hearing fairy tales when you were kids? Nice. Okay. Cool. Me too. I wanted to make sure because this talk is not going to be fun if you hadn't raised your hands. So that's good. So one of my favorite fairy tales growing up was the story of Goldilocks and the three bears. How many of you are familiar with that fairy tale? Cool. I see a couple of people who didn't raise their hands so very, very quickly Goldilocks and the three bears is a fairy tale where a girl named Goldilocks breaks into a house and it happens to be the house of three bears and the entire story is her basically like rummaging through all their stuff and using it like she lies down in their beds and eats their porridge and the Papa Bear's porridge is like too hot and the Mama Bear's porridge is too cold and then it turns out the baby bear's porridge is like just right. So the whole story is her like trying to find the just right thing. Which I kind of did a little bit of research on Goldilocks and the three bears and it turns out the story that we're the most familiar with was written in 1837, which is like 150 year old story, it's quite old. And looking back on it as an adult, I think it's probably strange to teach children to break into the house of bears. Like I don't know why it wasn't like bunny rabbits or otters or something more friendly. Bears are not friendly, but that's okay. But anyways in researching this, I got to thinking and it made me wonder what this fairy tale would be like if we did a modern day retelling of it. So that's what we're going to do today. But modern day Goldilocks is over the whole breaking into people's houses, bear's houses and using all their stuff. Modern day Goldilocks, she does something a little bit different. Modern day Goldilocks is a Rails developer because of course she is. So Goldilocks has started her first dev job and it's super, super exciting because she didn't used to be a programmer and she's still amazed that people pay her to write code. It's kind of an unfathomable concept to her. So her first dev job, she loves what she does because she gets to write amazing code. She thinks it's amazing. And she gets to work with a really great group of people and she's learning a lot. As we all know things change very quickly in the life of a programmer and after about six months she's writing code, but she doesn't really know if she's getting better at it. She doesn't know if she's building any technical chops. So she starts talking to some of her other programmer friends and she finds out that a lot of other junior developers are working at companies with formal code review processes. That's weird. We don't really have that at my company. My company is really small and usually if I want feedback I have to go and ask for it. I have to talk to another senior engineer and get him to look at my code. And the more she talks to other people she realizes that she doesn't have any written documentation of whether she's getting better or not because she doesn't have a code review process at her company. This is probably not a great thing. And after a while she starts to get really, really frustrated by this. So she decides maybe it's time for her to move on. And Goldilocks goes in the job hunt and she finds her second dev job working in Rails and also Angular but that's a different story. She's having an amazing time at this company and part of the reason for that is because everybody has code reviews. Everybody reviews code and everybody is reviewed. In fact on her first week on the job she reviews the CTO's code. She was like what? Me? Really? That's really cool. People think that my opinion matters and I can actually have opinions about technical decisions. This is really excellent. This is also a very small company. It's a startup. So they basically have one rule of thumb. As long as one person okays the code it can be merged in. This works okay. Code gets merged in and the team is really productive. However, Goldilocks does know that there are some drawbacks to this. Sometimes the code reviews kind of devolve into long strings of comments. Sometimes they're having discussions about hash rockets and whether or not we should use them which she feels like maybe we shouldn't do in code reviews. Sometimes they're discussing things like syntax so she goes ahead and she's like okay we're going to implement RuboCop because we shouldn't talk about these things. So it's not perfect but it's a start so that's good. And nothing terrible has happened really until one day. So Goldilocks is working on an amazing feature. She's been working on it for two weeks and she finally creates this amazing massive pull request and it gets okayed by two other developers on the team and it gets merged in. And she breathes a sigh of relief and she's like okay I'm going to go get coffee. I'll be back. She comes back 20 minutes later to find that there's a slight problem. And when I say slight problem I mean it's like a fire because all of staging has gone down and everybody's like what happened Goldilocks what did you do? Well I don't know you guys reviewed my pull request. I don't know what I did but Goldilocks not to be deterred she's going to fight this fire. She takes a look at her pull request her massive pull request and it turns out that she did some bad copy pasta and she has two files that are named the same two controller files and one of them is blank and was supposed to be deleted and it didn't get deleted and now the entire app has come crashing down because she has two conflicting files. The thing is nobody caught this during review so oh no. We thought we really had this code review thing down as a team. Just kidding we didn't we're clearly doing something wrong and this gets Goldilocks thinking okay there must be a better way of doing code reviews right Goldilocks is pretty audacious and she does what any developer would do in this position she does some research okay she does a lot of research and she starts with one of her favorite books code complete this ancient text dates back to the year 1993 and was written by Steve McConnell she's actually reading the second edition which came out in 2004 but that's fine but what she finds from Steve McConnell's work is that code reviews are something that aren't new and there's actually a lot of theory behind them and so she decides to dive into the theory in order to try to understand what they're supposed to be in the first place so while reading his book she comes across one passage Steve McConnell writes one part of managing software engineering is catching problems at the lowest value stage that is at the time at which the least investment has been made and problems cost the least to correct okay that's cool she thinks to herself I buy this in theory this makes sense I am on board she keeps reading and she finds that a lot of times code reviews are used as quality gates that is to say gates to make sure that after periodic testing and periodic review only code that improves our product improves our project and code base is what's getting allowed through the gates and nothing else okay but our quality gates my team's quality gates they're not they're not really functioning as gates they kind of feel like formalities maybe how did that bug get through code review she keeps reading and she finds that there's something called the collective ownership of construction and that's kind of like the cornerstone of where code is code reviews came from studies have found that the collective ownership of construction has a lot of benefits three main ones first is that oh that's a crazy thought bubble ignore that the first is that multiple sets of eyes on a code base always lead to better quality code another benefit is that if somebody leaves the team you have fewer fires to fight less impact because more people have seen the code so even if someone leaves someone else can pick it up and the third is that those defect correction cycles the cycles of debugging and fixing what's wrong with our code those cycles happen a lot faster because it's not just that one person knows what the bug is it's not just that one person knows where to start many people have worked with this code that's a great added benefit and it turns out that there are also three types of code reviews Steve McConnell's book talks about this in depth but the three types of code reviews still are seen in software development practices today in fact some of you might use them on your teams the first is something called inspections inspections actually date back to a team at IBM which was led by a guy named Michael Fagan and he started doing inspections on his team and when he saw them working so well internally he started sharing them with the rest of the community and what an inspection is is kind of like a really intense code review process the focus is on defect detection not correction this is really important because if you're correcting code as you go along you're not actually scanning it for any possible defects a core part of what inspections are are having a checklist of what you're looking for and having roles for all the people involved so what this team at IBM used to do was they would have at least three people at every code review a moderator the author of the code and a reviewer and all three of them knew what their role was and they all came into this meeting with a checklist knowing what they were going to look for in the code review process the most important part about this was that they would take any data anything that was working or wasn't going right and they would feed that back into the next code review process in other words they were perpetually iterating on their code review process in using inspections 60% of defects can be caught and one team that use inspections regularly found that 20 to 30% fewer defects fewer bugs were found per 1000 lines of code so they can work but not everybody has an hour necessarily that they want to spend on you know five or ten lines of code which leads to the second type of code review shorter reviews so the idea here is that even one line of code should be reviewed every single line of code should be reviewed and one team that was using that introduced short reviews before they introduced this process they had a 55% error rate in their programs after implementing short reviews that dropped to 2% that's just a 2% error rate when you start looking at less than five lines of code another team had a 86% correctness rate after they started implementing code short code reviews regularly that went up to 99.6 percent and the third type of code review that has been used throughout different software development teams is something called a walkthrough and this is generally like a working meeting 30 to 60 minutes long and usually there's a senior developer and a junior developer and it's supposed to be an exchange of ideas the senior developer will walk through and talk about what might happen if this program runs and try to look for any defects for any bugs and the junior developer is meant to ask questions and push back on any methodologies the senior developer might be suggesting and ask why these are a little bit less helpful and as Goldilocks is reading about this she realize oh this is exactly what I used to do at my first company but this wasn't very formalized and we didn't really have a plan for it we just kind of did it whenever but it is a thing when she looks at the peer sophistication peer review sophistication scale Goldilocks starts to feel really disappointed because she realizes all the software teams that I've worked on as awesome as they've been I don't really think we have ever really gone beyond a two on this am I the only one who feels this way well theory is great but it's very different than practice and being a programmer Goldilocks has no choice but to try to debug her team's code review process and in our modern detail of Goldilocks as a developer she does what any well-meaning developer who doesn't really know where to start she does what they would do she asks Twitter obviously because all good things come from Twitter she creates a survey and she spreads it throughout all of her colleagues and asked them to share it hoping that she can get some sense of whether everybody else is having the same feeling of disappointment with their code reviews as well a little disclaimer here Goldilocks is by no means a data scientist she probably could have done this survey a lot better and one major flaw with all of her data is that it's limited to whoever actually self-selected into the survey and probably in future iterations she'd like to improve this but it's a good start got to start somewhere what she finds is that of the around 500 or so people who answered and responded to her survey majority of them worked in Java Ruby and Rails and JavaScript okay so what do they think about code reviews this seems like a pretty diverse said maybe not the best but a decent number of people what do they think about code reviews she asked them to what extent do you agree with the statement code reviews have made me personally a better developer everybody seemed to agree with it they averaged about 8.6 on a scale of 10 interestingly she found that Swift developers found code reviews the most beneficial she's not sure what that means but Ruby developers were a close second they came in at about 9.2 on a scale of 1 to 10 she also wondered if she was the only one who had to ask for code reviews in her in her previous experiences or if everybody else also just had that same experience turns out the majority of developers who responded to her survey did have most of their pull requests reviewed but you'll notice that around 10% of respondents said that they always had to ask for review from their peers it wasn't something that was really planned or formalized or structured they had to reach out and get the review from someone else and this was also the case across languages this led Goldilocks to believe that a good code review process has nothing to do with what language you work in or with what framework you use and it has everything to do with your team and how you carry out your code she also found that most people only needed to have one person review their code before it could be merged into the code base and that people spent on average between five to 20 minutes reviewing a single pull request but they sometimes had to wait an hour or a day and maybe even a week in rare cases for their code to be reviewed and merged into the code base and it seemed like a lot of people were doing reviews but based on their responses not that many people were super thrilled with how they were doing them one person wrote in to her survey and said we have one developer who blindly thumbs up every pull request and rarely leaves comments they're attempting to game the rule of at least two approvals it's easy to tell because inside of one minute they've approved two to three pull requests someone else wrote in and said the second and third reviewers are way more likely to rubber stamp after seeing one other person's approval and this got her thinking are we just going through the actions of reviewing is it actually having an effect and for those people who did feel happy with their code review processes what was working what can we learn from their experiences and pull into our own team she thought well after reading 500 plus responses she found that almost always what makes or breaks a code review comes down to two things energy and substance so energy what that really means is who is doing the review and how much time are they spending on it she found that it's not just the act of doing a code review that matters it's equally about who is doing them and how much weight they carry on team someone wrote in and said everyone on the team should receive equal review I feel like it's so common for senior people to get feedback because people assume they can do no wrong they totally can and they might want feedback junior people get nitpicked to death and we should remember that people's self-esteem is likely to be affected remember that they're being vulnerable this really sat with Goldilocks and she thought about it she found somebody else who wrote in that code reviews need to be acknowledged as a first-class citizen in a dev team and they need to be thought of as a legitimate activity that need time and effort overall the overarching themes that she found were that big commits large pull requests with no context made for a really poor code review experience the same went for unequal reviews if junior developers were getting reviewed and never doing the reviewing or if they were never getting to see what senior developers were working on they generally walked away from those teams feeling pretty disheartened and overall developers who felt that their code review process was lacking generally pointed to the fact that either management or the team lead or somebody at the upper echelons could not justify code reviews get it being a first-class citizen they were always treated as less than important and not worth actual time okay but what about substance well when we talk about substance what we mean is what exactly is somebody doing while reviewing and what are they saying and how are they doing it even if everybody on the team is reviewing code what they're doing as a reviewer really matters and Goldilocks found some interesting data which was she noticed a lot of people were using the phrase nitpick she did a quick like find and file and found that over 5% of the people who responded actually referred to or associated the word nitpick with their code review process which probably says a lot someone wrote in and said that you should review the code not the person let tools handle styling changes for you so that you don't have to spend time with that another person wrote in and said if I ever find myself going back and forth on something with via comments I'll just ping the other person and ask them to pair sometimes in my team it's just really hard to talk to someone and another person wrote in and said sometimes on my team there's trolling of code and we haven't implemented any kind of code of conduct there were some interesting themes that came out of all of these responses when it came to substance which is that code reviews that devolved into tons and tons of comments that were nitpicky and focused on tiny details really should have probably been pulled out into conversations it probably means that you just need to talk to someone on your team and have a conversation or a discussion with everyone else a huge huge gripe for developers is that style and syntax end up being major pain points in code reviews someone wrote in and said most of our code reviews all turn into huge discussions about what to name a variable it turns into correction rather than defect detection as what code reviews should have been it's what they originally the theory behind them was to be rather than style and syntax being the focus if content and semantics were what people focused on developers ended up feeling like their review process was actually helpful and making them better engineers the biggest one though was code reviews that were very egotistical and not empathetic this goes for both the person doing the reviewing and the person being reviewed as Goldilocks looked through all of this data she realized that she wasn't the only person to have been disappointed by code review processes in the past everybody else seemed to have some issue with it or another and she started thinking back to what she read and what she had found in her research in the days of your IE like the 80s 25ish years ago code reviews were yes they were a checklist but they were iterative processes you were perpetually feeding data back into the code review system in order to figure out if what you were doing as a team was working or not and if it wasn't working you talked about it and you changed it and this kind of became the crux of what the problem was she found the bug in her team's code review process which was that they had stopped discussing it they had stopped sharing whether this was working or not they had stopped iterating on it and they weren't reflecting on whether their code review workflow was efficient and impactful so stories are great and fairy tales are wonderful but at some point you kind of have to like find the moral of the story or bring it back to reality because stories are stories the good news is that I told you this was a fairy tale but it's not actually a fairy tale because it's very much my story I am Goldilocks surprise plot twist anyways these days I don't work at either of those two companies I work at a company called Tilda and we build skylight your favorite rails profiler and because this isn't a fairy tale it's real life the truth is I don't actually know if we have our code review process right but what I do know is that we're still talking about it we have conversations about it and every engineer on our team feels like they can bring it up in conversation and actually make little changes to try to make the process better for them and for us as a team so if I've learned anything from this whole project of trying to understand what code reviews are and trying to learn from my peers about it it's these five things first to air is human and we are all human so we should review everyone and everyone should be reviewed second it really really helps your teammates if you can be concise and give context a couple of my colleagues that I've talked to have actually started using screenshots in their pull requests and detailing out what what the behavior was before the pull request and what it should be after and if it's a bug showing your teammates how to check it out how to check out a branch and actually like see the bug in action and see what your change did to change to impact the code the third is being kind I think there's really no better way of doing that than being able to leave your ego behind at the door and realize that this is the collective collaboration to construct something that's bigger than all of us as a team this is really hard to do and I think it really depends on each team individually fourth make it easier on yourselves use linters do not talk about tabs versus spaces because man that is going to get under somebody's skin implement linters and use templates if possible I know GitHub has a great feature where you can have a template when you open a pull review pull request and it will actually like go through and make a checklist for you and you know exactly what should be in that pull request and what's missing they're really awesome I encourage all of you to check it out and fifth if there is nothing else you remember from this talk I hope it's this iterate it's so important I'll say it again iterate and what I mean by this is talk about your code review process and perpetually keep it in flux because it's not a fairy tale the story is still going you can still make it better if it's not where you want it to be and if it's good you can improve upon it and share it with other people and other teams I think my favorite quote from all the people who wrote in to my survey was this I love code reviews in theory in practice they are only as good as the group that's responsible for conducting them in the right manner it doesn't really matter what language you use or framework or how big your team is or whether you're a consultancy or a product team it's on you as a team because all of you are adding to this amazing creation together so it's on you to make sure that your quality gates are actually of good quality so I hope that you leave today if you haven't started this conversation wanting to start it with your team and if you feel like code reviews aren't working for you talk about it and share your ideas and this data with them in fact if you want to check out this data you can visit better code reviews all of the data I'm not a data scientist but all of the data that I could manage to collect is up there and I did read a couple of people's responses there are tons and tons and tons of them they're really good I encourage you to check them out way more than I could fit into 35 minutes and if you'd like to continue the conversation you can add to the survey and if you're a data scientist maybe you can help me the data and you can contribute by bringing this back to your teams and starting the conversation because as much as fairy tales are great this one is just beginning that's all I have thank you so much