 Hello Singapore. That was good we can do better. Hi Singapore. Thank you. I was a little bit worried about this time slot because I know the only thing standing between me and your Friday hug from Erin is this like 30 minute talk but then I started thinking about the fact that the only thing standing between you and your beer or preferred evening beverage is Erin's talk and then I felt better so with that out of the way. Hello my name is Vaidehi and just to get a show of hands in the room. How many of you grew up either reading or listening to fairy tales? Okay it's a good number. Me too. How many of you are familiar with the story of Goldilocks and the three bears? Okay sizable number for those of you who don't who might not be familiar with the story to summarize. Basically there's this girl named Goldilocks and she breaks into a house and the house is the home of three bears and she basically like goes through all of their stuff which seems like a terrible thing to teach children because like if you're going to break into anyone's house it should probably be like you know puppies or bunny rabbits I don't think bears are the most friendly animals to do that to but that aside she basically goes through all of their things and she like has the mama bears porridge and the papa bears porridge and one is too hot and one is too cold and the entire story is her trying to find the thing that is just right. So I got to thinking about what Goldilocks's story might be like if it was reimagined and if we were to do a modern day retelling of it. So surprise it's story time we're gonna do a modern day retelling of Goldilocks and the three bears. However in our modern day retelling Goldilocks is over you know breaking into large furry animals houses instead she is a Ruby developer because of course she is. So Goldilocks is at her very first dev job ever and she is getting to build code and she's getting paid for it and she's like oh my god this is amazing I'm learning so much and she's working at a small consultancy and she basically has at her disposal all of these engineers that she can ask her questions to and she's like this is great amazing I'm gonna learn a lot and she does learn a lot and she has a lot of fun but after a few months she's definitely building things and she's contributing to projects but she's not really sure if she's getting any better at programming is she writing better Ruby code she doesn't know and she starts talking to other developers and she realizes that at her company she doesn't really get any written feedback and part of the reason for this is that she doesn't really get real code reviews her team is really small and if she wants code review she has to ask for it and she has to sit down with another engineer and tell them hey can you look at my code and can you tell me what could be better and what what I'm doing well and she finds us a little a little frustrating because she knows that other companies do have a bit more formalized code review processes so after about a year there she decides that this company is not the best fit for her anymore so she goes to another company hoping that she'll find what she's looking for there at her second dev job things are very different everyone is code reviewed and she even gets to code review in fact at her second week on the job she reviews the CTO's code and she's just like what I can have technical opinions turns out she can and she's really excited about that but even at this small startup that she's at there are some limitations so one of the limitations is that there's not very many people and so as long as one person reviews a pull request it gets merged in and this tends to be fine and code gets merged in keeps getting merged in lots of pull requests but there are some drawbacks one of the things that she notices is that her team will sometimes create pull pull requests and they'll devolve into long comment threads and sometimes people will be like we should use tabs no we should use spaces we should use hash rockets why are we talking about this and it'll just keep going and they're not always the most fruitful conversations but things are fine code keeps getting merged in and things are getting shipped and deployed and it seems to be working fine until one day so Goldilocks has been working on this feature for about two weeks and she has poured her heart and soul into it and finally she submits a pull request and two engineers give it a thumbs up and it gets merged in and she's like you know what okay I'm gonna go for coffee I'll see you kids later she comes back 20 minutes later to find that the entire engineering team is freaking out and there's a fire and the fire is that the entire staging environment has just like gone down in flames and they're like Goldilocks what did you do you just merge this giant feature in you did something and she's like well you reviewed my code both of you reviewed my code so I don't know what I did why didn't you see it but not to be deterred she decides that she's gonna fight this fire and what she finds out is really interesting turns out in this very large pull request of like 20 some files she had added a controller and turns out somewhere along the line of rebasing she had two controllers and one was empty and one was not and the empty one was overriding the one with all the code and the entire application has come crashing down so by committing an empty file that no one notice she basically caused kind of a mess so this gets her thinking oh we really thought that we had this code review process down no nope nope nope we don't because clearly if we had it down perfectly we wouldn't have had this problem occur I wouldn't be fighting this fire so she starts thinking okay there must be a better way of doing code reviews right right so Goldilocks being the Ruby developer that she is does what probably any of us would do she does research and she asked the internet and she reads about this ancient text called code complete which dates back to the old year of 1993 it's a really great book it's written by Steve McConnell I highly recommend that you read it and in reading code complete she starts learning about the theory in the history of code reviews and why we started doing them as teams and as an industry in the first place and in code complete Steve McConnell talks about a lot of different like research that he's done and the theory behind code reviews and one of the things that he says is that code reviews one part of managing a software engineering process is catching problems at the lowest value stage that is at the time at we at which the least investment has been made and the problems cost the least to correct and basically what he says is that code reviews are meant to act as quality gates just as we test and review the products that we build code reviews are meant to do that for our code bases but as she starts thinking about the teams that she's worked on the two teams she realizes that none of the code review processes that she's been a part of really feel like they're of good quality they kind of just feel like formalities so she keeps reading and she learns about something called the idea of collective ownership of construction and this is what code reviews are meant to center upon this idea that if all of us have a hand and a stake in what we're building together then our code will actually improve as a consequence and Steve McConnell has done a lot of research and he's found a lot of studies and he writes about this in code complete and she learns about a few of the different benefits that code reviews actually give us on our engineering teams one of those is that if you have multiple sets of eyes on a code base you actually end up with better quality code seems obvious but for those of us who might not do code reviews regularly it actually is a huge factor another thing is that if someone leaves the team and you have a code review process that's formalized it's a lot easier to pick up where they left off if someone leaves the team and everybody's at least looked at the code in review even if they've never worked with it before it will be a little bit easier for them to dig their teeth into it if they need to add a feature or fix a bug finally another great benefit of having code reviews is that the process of detecting defects the process of catching bugs goes a lot faster if someone's at least seen the code before or reviewed it if you've never seen a file ever and you're trying to find a bug in it it's a lot tougher so it turns out that in the late 80s a time that I you know can't really imagine anymore but back then code reviews came in three shapes and sizes one was something called inspections and this is actually something that was started by a guy named Michael Fillion at IBM and basically what he did was he started this hour-long code review process where there would be three people a reviewer the author of the code and a moderator and they would basically sit down and they would focus on defect detection they would focus on going through the code and looking for things that could potentially go wrong basically trying to snag bugs the important thing was that they were not correcting as they went along and there was a great talk this morning about not fixing bugs while refactoring so that same point it's really good to not fix bugs while you're reviewing instead keeping the review process as its own thing is really important according to Steve McConnell's research so these really long code review processes called inspections took a little bit of upfront time and effort but it turns out that they were actually really beneficial to the teams that use them the three people the reviewer the author of the code and the moderator would come in with a checklist and they would all know what they were looking for and what their roles were in the code review and when IBM started implementing this and when they shared it with other engineering teams it turns out that before using inspections and after 60% of defects in code bases were caught and there were 20 to 30% fewer defects per 1,000 lines of code after using inspections on teams the most important part of what these code reviews used to be was that after a code review process everybody on the team would sit down and basically talk about what went well and what didn't and they would take that data and recycle it back and they would iterate so that each time they did an inspection it would get better with time but not everybody has a thousand lines of code that they're adding to a code base sometimes you're adding only a few lines this brings us to our second type of code review this is something called short reviews and the basic idea here was that a lot of teams weren't doing any type of code review process for one line changes or five line changes and it turns out that based on research those five line changes and one line changes are actually the ones that are the most painful and the most error prone teams that introduced short reviews found that they would go from a 55% error rate to just a 2% error rate and those that didn't use short reviews and then implemented them found that they had an 86% correctness rate before having short reviews and then afterwards they had a 99.6% correctness rate so that's the second type of code review the third type is one that Goldilocks seemed kind of familiar with and this was called something called a walkthrough and this is effectively a type of working meeting where for 30 to 60 minutes you have two engineers usually a junior a more junior engineer and a more senior engineer and they talk about the code that's being submitted for review and technical choices and details that have been made and how to make them better this was not nearly as effective it is helpful but not nearly as impactful as a formalized kind of code review teams that use walkthroughs found that they could catch approximately 30% of errors in a program which is fine but not the most effective and the idea with walkthroughs is that they're a great learning opportunity for junior engineers to ask questions of senior engineers and for them to also give pushback against ideas and kind of like propel the team forward Goldilocks also stumbled upon something called the peer review sophistication scale and she kind of looked at this and started thinking about whether any of the code reviews that she had been a part of were actually of any good were they even valuable and as she thought about this and looked at the scale she realized that most of the team she's worked on she probably felt it number two maybe a number three and she got a little bit worried and she wondered am I the only one that feels this way does anybody else feel like this too and this is all theory but like in practice what are other teams doing do any of us have this code review thing down so she decided to debug this whole idea of code reviews in the industry today and because Goldilocks is a modern-day Ruby developer she did what any of us would do she asked Twitter I don't know if this was you know the best decision for her but she did it anyways because we know all good things come from Twitter and she basically created a survey and asked all the developers that she knew to spend a couple minutes and answer a couple of questions about what their code review processes were like and whether they were happy with them at all or not so little disclaimer Goldilocks is definitely not a data scientist so she probably could have done a better job of collecting data and analyzing it but it's a start one of the ways that this data is a little bit limited is that it's obviously going to only be people who answered the survey so it's not a great representation of all developers but it's something that we can use at least so the first thing that she learned is that the majority of people answering her survey survey worked in Java I don't know why they also used Ruby and Rails and JavaScript and there were some other languages as well but for the most part that was the majority of people who were answering the survey so something to keep in mind and she found that the majority of people who responded did feel like code reviews made them better developers so they seem to be on the same page as her that code reviews were overall a good thing okay cool we all seem to be on the same page what else can we agree upon well it seemed like Swift developers found code reviews to be the most beneficial they came in at about a 9.5 on a scale of 1 to 10 and Ruby developers came in second so not bad 9.2 so everybody seemed to think that code reviews were helping their teams what was interesting was that while most people did have all their pull requests reviewed for their teams there was still a whole 10% of people who had to ask for review 10% of respondents didn't have any type of formalized code review process they had to ask for it which made her feel a little bit better because she had been in that position before too what was even more interesting was that it didn't really matter what framework you worked with or what language some teams seem to have a formalized code review process down and others just didn't which led Goldilocks to believe that it whether you have a code review process that works has nothing to do with what language you use or what framework and everything to do with the people on your teams another interesting thing that she found was that like her startup most teams only needed one person to review a few people needed more reviewers but generally one reviewer seemed to be enough however those reviewers would spend only 5 to 20 minutes doing a review of a pull request and there are some people who who had to wait up to a day or even a few days to receive review from their teammates so as she read through some of the responses in her survey she saw that a lot of people were doing code reviews but not all of them were happy with how they were being conducted and she kind of boiled it down to two aspects with two things that made made or broke a code review work time and substance one person wrote into her survey and they said we have one developer who blindly thumbs up every PR 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 three pull requests someone else wrote in and they said if the second if there's a second and third reviewer there are more likely to just rubber stamp after seeing the first person's review so Goldilocks started wondering are we just going through the actions of reviewing each other's code like are we considering time and substance because it seems like a lot of us are unhappy with this so let's figure out what we mean when we say time and substance the amount of energy and the amount of time that you spend on a code review turns out to be pretty significant in how happy engineers are with the code review process so what we mean by this is who's doing the review and how much time are they spending on it because that seems to really make a difference it's not just the act of doing the code review it's how much weight they carry and who's actually doing the work someone wrote into her survey and they said everyone on the team should receive equal review I feel like it's so common for senior people to get no feedback because people assume that they can do no wrong they totally can and they might want feedback junior people get nitpicked to death and people seem to forget that self-esteem can be easily affected and that when someone submitting a code review they're being vulnerable another person wrote in to her survey and said that code reviews need to be acknowledged as a first class citizen they are a legitimate activity that needs time and focus and as she read through these responses she noticed that there were a couple common themes that kept popping up developers seem to be really frustrated by code reviews that had very large commits or really long PRs and those that provided no context it meant that you would have to spend more energy and time on the code review and that seemed to be a huge barrier it was also very frustrating for developers when different people were spending different amounts of time reviewing particularly when junior developers would get more criticism and longer reviews and senior developers wouldn't and another point of frustration was when technical leads or upper management or CTOs would basically prevent you from as a team treating code reviews as first class citizens when code reviews were downvalued developers felt more frustrated by their team's overall process and productivity so what about substance well as it turns out even if everyone reviews and is reviewed what they're saying and doing while reviewing is pretty important so when we say substance what we're really talking about is how a code review occurs so Goldilocks did a like a find and file in all of her responses and she found that five percent of people just automatically had a negative reaction with the phrase code review and they were they basically use the term nitpick like I hate code reviews they're so nitpicky they're terrible and that was like five percent of her respondents just automatically which really spoke volumes about how people feel about code reviews and what they actually are doing to the team's morale so as it turns out there are a couple things that seem to be points of frustrations in points of frustration for what a code review consists of people really did not like tons and tons of comments on their pull requests they would much rather have a conversation about it they also seem to be really frustrated by having conversations about style and syntax and that seemed to be an indicator of a negative code review process they did however like having conversations about content and semantics those seem to be indicators of a more positive code review process the biggest one however was people who had empathetic code reviews generally seem to be happier with their workflows whereas people who had experiences with very egotistical reviewers or submitters of code reviews were more frustrated by the entire process and thought of it in a very negative light one person wrote into her survey 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 this as a team another person wrote in and said if I ever find myself going back and forth on something via comments I'll just ping the other person and ask them to pair sometimes it's just a lot easier to talk to someone another person wrote in and said that on their team there was trolling of code and they didn't even have any kind of code of conduct so it's really hard to stop this and this was both disheartening but also kind of uplifting for goalie locks because she realized that she was not the only person that has had gone through this she was not the only person who had been disappointed by code reviews in the past and in reading through her survey responses she realized that one of the major factors of what had changed between 25 years ago when Steve McConnell wrote code complete and talked about the theory of code reviews and engineering teams today was that a lot of teams didn't have any concept of a checklist of what a code review should or shouldn't be and even worse they had stopped collecting data and iterating on their code review processes people had stopped talking about it and people had stopped thinking about whether it was actually effective they were just going through the motions of doing them so I told you that this was a fairy tale and we've been going along this journey with Goldilocks but I have been not entirely truthful with you because as it turns out this is not a fairy tale I'm Goldilocks surprise haha plot twist I know nobody saw that coming so these days I don't actually work at either of those two companies I work at a company called Tilda in Portland, Oregon and we build something called skylight which is your favorite rails profiler if you need something to profile your rails application you should check us out and to be totally honest with you I don't even know if we're doing code reviews right but what I do know is this we have a lot of conversations about them and every engineer on our team feels like they can talk to the rest of the team and bring up issues or points of frustration if they have them one of my coworkers actually she submitted an RFC to totally change our code review process and that RFC was reviewed many times with a lot of comments but we decided to adopt and that's now the process that we use as a team so I can feel like I can bring up any issues to the rest of my colleagues and we can talk about that and talk about them and iterate and change them so based on all of this research that I've done and all the people that I've talked to and the survey I wanted to share with you a couple of things that you can do small and big to change your code review processes if you're not happy with them because clearly as an industry we all have some work to do on this so a couple of small wins that you could implement when you leave here today commit hooks are really awesome they basically make sure that you can run whatever tasks you need to before you push anything to GitHub they'll make your life a lot easier I recommend trying them out GitHub also has an amazing feature called templates and we use this at Tilda where basically when somebody opens a pull request this template shows up and it has like a checklist of things you need to do before you can submit a pull request things you need to do before you can ship and it's really great because people can immediately look at it and know whether something needs to work still or if something's ready to be reviewed another thing that's super super helpful for your teammates especially if you have a large team or you have people who might be more comfortable with certain parts of the codebase is giving them context giving your teammates context makes it a lot easier for them to target their efforts and Don gave a great talk today about how human beings have like a certain capacity to focus and hold things in memory this is going to make it a lot easier for your teammates to be able to focus on what changes you made it's really helpful to like add a gif or screenshot and show them like what is the thing that happened before the pull request and what can you expect to happen after it'll make the process a lot easier and we'll probably put people in a much better mood linters use them we don't need to talk about like syntax and stuff like those are conversations that we don't need to have because we have machines to automate those things for us so definitely use them and implement them tiny win but really really impactful and lastly encapsulate everything whether that means working on writing concise commits or creating small pull requests that are short and fixing things in a really really concise and compact way encapsulate things so that nobody's reading hundreds of lines of code and feeling overwhelmed and just giving it a thumbs up and moving on because that's going to come back to haunt us later some teams have also found that it's really helpful to assign specific reviewers particularly if you find yourself like waiting for someone to review a long time it's also really great because you can make sure that senior developers are reviewing and being reviewed and that junior developers also get a chance to look at code that a senior developer might be working on and ask questions and learn about things that maybe they might not be familiar with yet there are some big wins that you can do they might look a little daunting but I promise you the payoff is super worth it first pushing for a culture that values vulnerability and I think one of the best ways to do this is by making sure that everyone puts themselves out there senior developers can make mistakes I'm going to say it again senior developers can make mistakes and it's important for junior developers to see that and to be able to ask questions when we're working in teams that are vulnerable enough to acknowledge that we all become better developers and create a better product at the end of the day develop empathy this is harder to do but you can do tiny things to make sure that your teammates are becoming more empathetic engineers and people one of the ways that I really like to do this is by calling out the things that I see another developer doing really well so if there's a method that's named really beautifully or some sort of abstraction or something that's been met a program I really like to call that out and say that you know you did a great job that was awesome that's so important because code reviews don't need to necessarily be something that's negative and you know a symbol of criticism we have this collective ownership here and what we're building so when someone does really well that's all our victory in that too and finally iterate if you remember nothing else from today if this is so important I'll say it again iterate and what I mean by this is start the conversation if you feel like your code review process isn't where you'd like it to be and if you feel like there's room for improvement or if you're really happy with it share it with other people this someone wrote into the survey and I think they they highlight this way better than I could ever could it's a really eloquent quote and I'm not nearly as eloquent they said 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 really it's on all of us at the end of the day to make sure that our quality gates are of good quality so if any of this data was interesting to you I promise I'm not a data scientist but I really tried you can find all of it at this website better code dot reviews there are a lot of people who wrote into the survey and they have some really amazing on thoughts on what makes a good code review and what doesn't unfortunately it's way more than I could fit into 30 minutes but you can find it all there and you can take the survey yourself and tell me your thoughts on code reviews I would love to hear them and that's all I have for you thank you so much all right with anyone like to ask all the locks a question anyone yeah yeah thank you for the talk I really appreciate the research you put into it it's it's definitely awesome so my company doesn't have a code review process but we do do pair programming like the majority of the time and that seems to address some of or at least attempt to address some of the some of the quality issues that you wanted to bring into code reviews and like communication in person communication so does Goldilocks have an opinion on pair programming she loves it I don't think I can do the rest of this question in third person sorry I'm a really big fan of pair programming we do pair programming at tilde so we're always working with pairs and switching them up but I think what's really nice about it well what's really nice about having a code review process even if you do have pair programming is that when someone for example if I'm working with my pair on a feature and someone else is working on another part of the code base it's nice for them to be in tune with what changes are happening in a different part of the code even if you know it's like at the end when they're submitting you know a feature PR or something like that because things get merged into the code base all the time and it can just kind of go as like a stream of things on a slack channel and you don't know what's happening so I would still really encourage some kind of code to be processed it doesn't necessarily need to be like an hour long inspection or something but I think it's really beneficial to see what's going on in the code and sometimes like you might even have some context to provide on what you're working on and be like maybe we should name this method slightly differently we did something me and my pair did something over here you might want to like check it out and it might be beneficial to the two of you on what you're doing so I still think that code reviews can be super beneficial but it might look different if you are working in pairs very question okay so love hands choose one yeah so I have a two part question the first one is and it's a direct follow-on to that what does Goldilocks think about bear programming okay no opinion or maybe you're just not allowed to talk about it so the next one is so what is your ideal checklist look like or what's your good checklist to go through because this would be useful for me so like what do I look for in a code review basically what's your list yeah I feel like I should probably write this down is a good question for me it's probably can I read the code and understand what's the problem trying to be solved if I looked at it again in six months is it clear to me if we left someone left the company and was if there was a junior programmer looking at this would they be able to understand like our methods name clearly are things actually doing what they're doing and are there tests that's a big one cool okay hi thank you for the great talk thank you have you thought about automate the small wins for code review sorry can you repeat the question one more time have you thought about automate the small wins for code review I haven't I haven't automated all of those things I know people who have I haven't personally been part of that I have I like implemented Rubo cop at my last company but I know other people who have like I think the most automation we have is like the PR templates and like having linters to enforce style things like that nice because I know one tool is called dangerous system that can help automate code review thank you hi here so first of all it's great talk I'd like to share about our company it's I think it's better than the average code review but it's not perfect so because the checklist that we're looking is if if the even if it's a bug fix sometimes if there's no test will ask where's the test something like that and yeah like you said screen shots and maybe how to how to test the feature and all that and I think it the code review process also depends on the people and I think in our team we we have our egos in check so we don't really get affected by the criticisms and but the problem that we are encountering is that for bigger features of course many people do review but when it comes to the merging part no one wants to press the merge button because no one wants to be responsible for if something breaks so because you mentioned about collective ownership so how do you think we can solve that problem so I'll tell you one thing that we do at my company that has really helped mitigate this which is that again we pair so one of the things I love about that is that you really displace this concept of blame that it's not one person who wrote a you know wrote something that they didn't write buggy code or something or they didn't you know break everything or bring down production like there are other people who were involved in it too and it's coding is not something you do in a silo and I think trying to create a team whether that means like having people work on one feature and break it into pieces and take it apart and like you work on this and I'll work on that or pairing on some tougher parts like if there's part of it the system that's more complicated having someone to work with work with you through that is really helpful but on a bigger level it means that it's never just like your fault and I think it's just something that speaks to something larger in the industry where this idea of placing blame and fault and like you broke everything and brought down production like that's that's never going to help the person or the team and I think it ties into this idea of like to create a more empathetic team you need to like change the language you use and code reviews need to change the idea that you know it's all your fault and it's all just on you whereas as you said collective ownership having everybody have a stake in it and have a hand in it that's a really great question I'd love to talk to you more about that but that's a really hard thing to do because it depends on your team a lot. Okay so one thing that we do when that happens sometimes we just paying those people on slack and talk it about it collectively but yeah even if you don't really blame people sometimes it's just a personal you know mindset that maybe I'll break this maybe they'll blame me even if they will not so I think it's a psychological problem as well so yeah I think changing those things are harder to do and developing empathy is not an easy thing but you can take tiny steps towards it but it takes it's a harder it that's why it's like a big win not a small win because it takes a longer time to do that. Okay I think we only have time for one more question so let's play a game of fastest hands up okay all the hands down okay all the hands up first so I know where to look can I get who wants us one two three with anyone else because just the three of you okay hands down when I say three the hands come up again okay one two three you cheated so it's you. So things like taking the time to do a thoughtful and empathetic code review and also taking the time to really contextualize the PR for the reviewer those take time what are some techniques you think that tech that teams can use to maintain momentum when they're pausing to put so much effort into empathy and documentation. That's a good question that was a hard one. Sorry. So I guess your question is like in making time to do code reviews how do you still make sure that you're productive to be honest I think a large part of it is making code review as a like submitting the review and doing the worker reviewing needs to be part of productivity because if you can do a better code review and avoid like eight hours of dev time debugging something that is productivity it's like preemptive right so part of that is like changing maybe upper management or the tech leads concept and kind of like selling them and making them a stakeholder and why code reviews are important but I think another thing that really helps is by breaking it down into small small pull requests and commits you make it easier like the barrier for entry of how to get through reviewing is easier because then you're not like oh I have this hundred line or you know 20 files thing to review I'll just save it for like 4 30 p.m. because I don't want to do it because I have my own work to do but if you make it a smaller thing that's more easy to digest it's something that you know if you ping someone they can find sometime in the next hour hopefully to do it like making it more bite sized I think is another good way to make that a bit more approachable and not something that slows you down as a team but I'm sure there are more ways but I have to think more on how to answer that question but it's very good thank you. Thank you. All right let's give it up for my day here. Thank you very much.