WEBVTT 1 00:00:14.679 --> 00:00:17.679 Hey everybody, and welcome back to another episode of Adventures 2 00:00:17.719 --> 00:00:21.199 in DevOps. This week, on our panel we have Jeffrey Girlman, Hey, 3 00:00:21.280 --> 00:00:23.160 check today doing all right? 4 00:00:23.480 --> 00:00:26.199 We also have Caleb Fernari, Hey, Chuck, how's it going? 5 00:00:26.760 --> 00:00:27.359 Doing well? 6 00:00:27.399 --> 00:00:30.120 I'm Charles Maxwood from dev chat dot tv. Just by 7 00:00:30.160 --> 00:00:32.119 way of announcement, we're starting a new show, or I 8 00:00:32.159 --> 00:00:35.119 guess I'm starting a new show. We released first episode 9 00:00:35.119 --> 00:00:36.920 this morning as we record this. It's called The dev 10 00:00:36.960 --> 00:00:39.960 Influencer Show. You can find it at dev influencers dot com, 11 00:00:40.039 --> 00:00:43.520 slash podcast, and I bet you can guess what it's about, 12 00:00:43.640 --> 00:00:48.399 talking about being an influencer in the developer and devop spaces. 13 00:00:48.439 --> 00:00:51.079 So if you're interested in that, go check out the show. 14 00:00:51.479 --> 00:00:53.560 I'm also doing an accelerator and you can find more 15 00:00:53.679 --> 00:00:56.039 about that at dev influencers dot com. We have a 16 00:00:56.039 --> 00:01:00.000 special guest this week, and that is James Donahue. James, 17 00:01:00.039 --> 00:01:02.520 do you want to say hello and introduce yourself all 18 00:01:02.520 --> 00:01:04.719 the way from the Britain and BBC. 19 00:01:05.079 --> 00:01:05.239 Hi? 20 00:01:05.359 --> 00:01:08.319 Yeah, James, I'm a software engineer at the BBC. I'm 21 00:01:08.359 --> 00:01:10.359 really happy to be on the show. Thanks for having me. 22 00:01:10.640 --> 00:01:11.480 Yeah, thanks for coming. 23 00:01:11.519 --> 00:01:13.519 I mean some of my favorite shows come out of 24 00:01:13.560 --> 00:01:17.200 the BBC, Doctor Who and Downton Abbey, which are very 25 00:01:17.280 --> 00:01:18.200 much not alike. 26 00:01:19.879 --> 00:01:22.159 Yeah, I have enjoyed those shows. 27 00:01:22.640 --> 00:01:24.959 Great British Baking Show is another one that started at 28 00:01:24.959 --> 00:01:27.400 the BBC. I don't think they own it anymore. 29 00:01:27.000 --> 00:01:30.400 But yeah, oh yeah, Roy Pokua, Yeah, everyone loves that show. 30 00:01:30.599 --> 00:01:30.920 Yeah. 31 00:01:31.079 --> 00:01:35.920 So you're you're like famous adjacent, right, we'll just say that, Yeah, 32 00:01:35.920 --> 00:01:38.079 you wrote this article on codeer Views. And what was 33 00:01:38.079 --> 00:01:40.799 interesting is is that we we lined it up on 34 00:01:40.840 --> 00:01:43.719 this show and MICHAELA was asking me what shows to 35 00:01:43.760 --> 00:01:44.920 get it on, and I was like, well, it's kind 36 00:01:44.920 --> 00:01:46.840 of good for any of the shows, and she's like, 37 00:01:46.879 --> 00:01:49.560 can I get it on the devop show? And initially 38 00:01:49.640 --> 00:01:52.680 I told her no. And then because because usually I 39 00:01:52.680 --> 00:01:55.560 see the article a name, I don't see the author name, 40 00:01:55.599 --> 00:01:57.159 and so you get on and I'm trying to make 41 00:01:57.200 --> 00:01:58.400 this connection, like what are. 42 00:01:58.319 --> 00:01:59.000 We talking about here? 43 00:01:59.359 --> 00:02:02.120 But I realized that we're doing a lot of infrastructuring 44 00:02:02.120 --> 00:02:05.159 code these days, right, whether it's hey, I'm going to 45 00:02:05.200 --> 00:02:07.879 go spin up this virtual server and then run chef 46 00:02:07.959 --> 00:02:10.879 or Puppet or something against it, or whether or not 47 00:02:10.919 --> 00:02:13.479 we have some provisioning software that goes out and manages 48 00:02:13.520 --> 00:02:16.319 our network or manages our servers, or whether we have 49 00:02:16.400 --> 00:02:18.840 something that manages things for Kubernetes or things like that, 50 00:02:19.159 --> 00:02:21.280 and so those code reviews are just as important as 51 00:02:21.280 --> 00:02:23.759 the other code reviews were doing. So I just kind 52 00:02:23.759 --> 00:02:25.360 of want to give people a little bit of background. 53 00:02:25.360 --> 00:02:27.800 This does apply to what you're doing in DevOps, but 54 00:02:28.280 --> 00:02:30.919 to get us started, I was looking at this article 55 00:02:31.039 --> 00:02:33.840 and you kind of explain, Hey, look, you know we 56 00:02:33.840 --> 00:02:37.800 we had to go remote, and because of that, you know, 57 00:02:37.879 --> 00:02:41.800 we experience certain things, and so we started doing the 58 00:02:41.840 --> 00:02:43.960 code reviews in a certain way. And I was wondering 59 00:02:43.960 --> 00:02:46.840 if you could tell that story, because I think this 60 00:02:47.039 --> 00:02:49.120 is something that people are really going to identify with 61 00:02:49.159 --> 00:02:51.520 that people are going to go, oh, yeah, we have 62 00:02:51.639 --> 00:02:55.000 that problem, and you know, maybe this will help some 63 00:02:55.039 --> 00:02:57.080 of them find that solution that they're looking for. 64 00:02:57.319 --> 00:02:58.599 Right, Yeah, of course, happy to you. 65 00:02:59.280 --> 00:03:01.919 So basically, the area that I work in at the 66 00:03:01.960 --> 00:03:06.080 BBC's the World Service, and we have international news websites. 67 00:03:06.680 --> 00:03:10.360 We have about thirty five million weekly users across the globe. 68 00:03:10.520 --> 00:03:12.800 The sites are in forty different human languages as well, 69 00:03:12.879 --> 00:03:14.800 so there's quite a lot of complexity in our code 70 00:03:14.800 --> 00:03:19.120 base dealing with all of that internationalization and the fact 71 00:03:19.120 --> 00:03:22.159 that it has a global audience, and we started rebuilding 72 00:03:22.199 --> 00:03:25.199 that application a couple of years ago. We had an 73 00:03:25.280 --> 00:03:30.240 old version that was a monolithic PHP applications service very well, 74 00:03:30.520 --> 00:03:33.800 but it was time to update the technology and we 75 00:03:33.879 --> 00:03:36.319 had lots of ideas new features we wanted to build. 76 00:03:37.039 --> 00:03:40.400 And what happened was we went in quite a short 77 00:03:40.439 --> 00:03:43.520 space of time from having about five developers working on 78 00:03:44.080 --> 00:03:49.120 a new renderer. We built a React based isomorphic single 79 00:03:49.120 --> 00:03:52.960 page application to replace the old monolith. I think we 80 00:03:53.000 --> 00:03:56.159 had about five developers working on it initially and over 81 00:03:56.159 --> 00:03:58.120 the course of really just a few months we went 82 00:03:58.159 --> 00:04:01.759 from that to about thirty five developers. And also another 83 00:04:01.800 --> 00:04:04.639 thing happened was that we went from all being based 84 00:04:04.680 --> 00:04:08.360 in one office in London at Broadcasting House to being 85 00:04:08.599 --> 00:04:12.400 distributed across different cities in the UK and also in 86 00:04:12.439 --> 00:04:14.800 other countries as well in Africa. We had some developers 87 00:04:15.439 --> 00:04:18.399 we were starting to have to get used to remote 88 00:04:18.519 --> 00:04:22.319 first development, and this is before the pandemic, this is 89 00:04:22.360 --> 00:04:24.720 before you know, the big changes of last year. We 90 00:04:24.720 --> 00:04:28.199 were having to adapt to scaling quite quickly and also 91 00:04:28.240 --> 00:04:31.319 being remote first all the time, and that's when we 92 00:04:31.360 --> 00:04:34.360 started to notice that something interesting was happening to code reviews. 93 00:04:35.079 --> 00:04:36.560 Some of our code views were taking out a bit 94 00:04:36.560 --> 00:04:38.720 longer than we'd like. We noticed there were some problems 95 00:04:38.720 --> 00:04:41.480 where code reviews would spend a long time going on. 96 00:04:41.519 --> 00:04:43.560 There'd be a lot of comments on the pull requests, 97 00:04:43.879 --> 00:04:46.759 a lot of back and forth between different people. A 98 00:04:46.800 --> 00:04:49.160 lot of new engineers were contributing to the code base, 99 00:04:49.279 --> 00:04:51.959 and not all of them really knew how we did 100 00:04:51.959 --> 00:04:55.279 code reviews or how we approached them, and we felt 101 00:04:55.319 --> 00:04:57.839 that there was probably a lot of tacit assumptions in 102 00:04:57.879 --> 00:05:00.360 the way we were doing code reviews. We also had 103 00:05:00.360 --> 00:05:02.040 a bit of feedback from some of our developers that 104 00:05:02.040 --> 00:05:05.199 they found the process quite confusing and quite stressful sometimes. 105 00:05:05.839 --> 00:05:07.800 And it was really at that point that we thought, Okay, 106 00:05:07.839 --> 00:05:09.399 we need to take a step back from this. We 107 00:05:09.439 --> 00:05:12.600 need to talk about what is this code review process for? 108 00:05:12.800 --> 00:05:14.319 Why do we do code reviews? 109 00:05:14.560 --> 00:05:16.720 And the interesting thing was that none of us could 110 00:05:16.720 --> 00:05:18.160 really remember ever having. 111 00:05:17.920 --> 00:05:19.199 Had this conversation before. 112 00:05:19.319 --> 00:05:22.000 We'd all been doing code reviews for years, and we 113 00:05:22.160 --> 00:05:24.040 never actually said why do we do this? 114 00:05:24.120 --> 00:05:25.439 What are we trying to get out of this? 115 00:05:26.000 --> 00:05:28.120 And that was really the starting point, we wanted to 116 00:05:28.160 --> 00:05:31.360 talk about it honestly and openly and really find out 117 00:05:31.360 --> 00:05:32.879 what everyone wanted to get out of the code of 118 00:05:32.920 --> 00:05:33.600 view process. 119 00:05:34.040 --> 00:05:34.720 That makes sense. 120 00:05:34.759 --> 00:05:37.680 It's funny too because as you go to kind of 121 00:05:37.680 --> 00:05:41.639 that remote first or primarily remote culture, right, because some 122 00:05:41.720 --> 00:05:45.920 companies went almost fully remote. In other companies they either 123 00:05:45.959 --> 00:05:49.600 had some kind of requirement or managed to have hold 124 00:05:49.639 --> 00:05:53.879 on to having some of their folks stay in the office, right. 125 00:05:54.240 --> 00:05:56.480 But as you move even part of your team out 126 00:05:56.480 --> 00:06:00.600 of the office, the requirements on your communication goal way up. 127 00:06:01.399 --> 00:06:04.560 And so it's it's definitely interesting that, Yeah, all of 128 00:06:04.600 --> 00:06:07.839 a sudden you're having discussions about how you communicate in 129 00:06:07.879 --> 00:06:11.000 these ways. I'm a little curious before we dive into 130 00:06:11.319 --> 00:06:15.480 how to start approaching this though, or other panelists Jeffrey 131 00:06:15.519 --> 00:06:18.680 and Caleb, I mean, what, what what's been your experience 132 00:06:18.720 --> 00:06:20.920 with this kind of thing, going remote in this way, 133 00:06:21.040 --> 00:06:22.240 especially through COVID. 134 00:06:22.560 --> 00:06:24.480 Yeah, I know, I think you sort of hit it 135 00:06:24.519 --> 00:06:28.079 on the head. I mean, it totally made us really 136 00:06:28.120 --> 00:06:31.120 refocus on how we communicate and and you know, you 137 00:06:31.160 --> 00:06:33.759 just can't walk over to the to the next person's 138 00:06:33.800 --> 00:06:36.199 cube or you know, or say hey, let's just grab 139 00:06:36.199 --> 00:06:38.279 a breakout room or something. I mean a physical breakout 140 00:06:38.360 --> 00:06:41.879 room that a zoom breakout room, right, and that changed 141 00:06:41.879 --> 00:06:44.680 a lot. But I'm curious. I want to i want 142 00:06:44.720 --> 00:06:48.240 to throw a question back to James. I'm curious what 143 00:06:48.360 --> 00:06:52.279 you guys came up with, because when I read your 144 00:06:52.360 --> 00:06:54.959 article on Medium, I was thinking, Wow, you know you 145 00:06:55.079 --> 00:06:58.319 ask all these questions about like why are we doing 146 00:06:58.399 --> 00:07:02.000 code reviews? And I was thinking, you know what you said, 147 00:07:02.000 --> 00:07:04.959 it's all the above, and you're like, what fifteen or 148 00:07:05.000 --> 00:07:08.319 twenty different answers? What did you guys come up with? 149 00:07:08.600 --> 00:07:11.519 What is the reason, what's the motivation? 150 00:07:12.000 --> 00:07:12.279 Well? 151 00:07:12.399 --> 00:07:14.879 Yeah, I mean you're right, we did list a lot 152 00:07:14.920 --> 00:07:18.040 of things that we thought code reviews before. I think 153 00:07:18.079 --> 00:07:21.160 in the blog post I put like fifteen different possible 154 00:07:21.199 --> 00:07:23.600 reasons that you could do code reviews. The thing we 155 00:07:23.720 --> 00:07:25.959 kept on coming back to time and time again, the 156 00:07:26.000 --> 00:07:29.519 motivation for code reviews that we seemed to be hitting 157 00:07:29.519 --> 00:07:32.839 no matter how we talked about it was communication and 158 00:07:32.959 --> 00:07:36.360 understanding a lot of the things that I summarized in 159 00:07:36.399 --> 00:07:39.600 the article really came out of discussions in our teams 160 00:07:39.600 --> 00:07:43.399 where we were asking what code reviews for? So you know, 161 00:07:43.480 --> 00:07:46.199 if you're if you're if you're discussing different approaches on 162 00:07:46.240 --> 00:07:49.040 a code review, that's a form of communication. If you're 163 00:07:49.079 --> 00:07:53.680 linking to other documentation or resources, that's communication. You know, 164 00:07:53.720 --> 00:07:58.000 you might be checking that work meets certain quality standards, 165 00:07:58.480 --> 00:08:02.079 or thinking about readability and maintainability, those kind of things. 166 00:08:03.240 --> 00:08:05.319 Communication seems to be the recurring theme here. 167 00:08:05.439 --> 00:08:08.279 So we felt that that was really something we like, 168 00:08:08.360 --> 00:08:09.839 we wanted to put at the heart of what we 169 00:08:09.839 --> 00:08:11.839 were saying that code reviews werefore because a lot of 170 00:08:11.839 --> 00:08:14.720 people have got different expectations about what code reviews are 171 00:08:14.720 --> 00:08:16.920 really for them. That and that kind of surprises people 172 00:08:16.959 --> 00:08:18.800 when you when you say to them, hate it's more 173 00:08:18.800 --> 00:08:20.439 about communication than anything else. 174 00:08:20.839 --> 00:08:23.879 Yeah, absolutely, I always felt like it was. There was 175 00:08:23.920 --> 00:08:29.720 almost this you know, unspoken idea that it's all about quality, 176 00:08:29.759 --> 00:08:32.279 code quality, and hey, it's just up to snuff. And 177 00:08:33.039 --> 00:08:36.120 you know, I mean, I've got a security background, so 178 00:08:36.320 --> 00:08:39.600 I'm in cybersecurity and application security all the time. And 179 00:08:39.639 --> 00:08:43.120 so when you know, folks in my world think about 180 00:08:43.240 --> 00:08:46.960 code reviewers, we're always focused on thinking about code defects 181 00:08:47.039 --> 00:08:50.159 that are of a security in nature. Right, how do 182 00:08:50.200 --> 00:08:53.720 we how do we try and find those underlying issues 183 00:08:53.720 --> 00:08:56.639 in code and sometimes just to go off on a 184 00:08:56.720 --> 00:08:58.759 tingent for a moment. I think that's interesting is that, 185 00:08:59.480 --> 00:09:02.039 you know, what, do a lot of reverse engineering of 186 00:09:02.159 --> 00:09:06.960 code to sort of understand, you know, where can somebody 187 00:09:06.960 --> 00:09:09.600 who's trying to do something nefarious, where can they sort 188 00:09:09.639 --> 00:09:12.960 of grab a toe hold and do something interesting, whether 189 00:09:13.000 --> 00:09:16.120 it's around access control, authentication, something like that, you know, 190 00:09:16.320 --> 00:09:19.600 in the software base. And so I just sort of 191 00:09:19.600 --> 00:09:23.519 gravitate towards that idea that code reviews are always about, 192 00:09:23.879 --> 00:09:28.240 you know, finding code defects. And it's interesting because I 193 00:09:28.279 --> 00:09:31.080 think there's a lot to what you're saying that it 194 00:09:31.159 --> 00:09:33.399 really should be a lot about just getting the whole 195 00:09:33.399 --> 00:09:36.320 team together and sort of communicating and you know, sort 196 00:09:36.320 --> 00:09:38.480 of more on getting them on the same page, because 197 00:09:39.240 --> 00:09:41.840 you know, at least generically speaking, I can see a 198 00:09:41.879 --> 00:09:45.039 lot of value in that and then doing you know, 199 00:09:45.120 --> 00:09:48.600 if you've got specific purposes like what I'm describing, then 200 00:09:48.639 --> 00:09:51.320 that might be a different use case, a different reason, 201 00:09:51.559 --> 00:09:54.840 and maybe a different setting for running a code review 202 00:09:55.200 --> 00:09:57.679 outside of I think what you're describing. But I don't 203 00:09:57.679 --> 00:09:59.240 want to put words in your mouth, but I'm sort 204 00:09:59.279 --> 00:10:02.000 of just trying to get an old Sheldon in my 205 00:10:02.080 --> 00:10:03.000 own mind. 206 00:10:03.440 --> 00:10:05.840 Sure, you know, I think you're absolutely right, Jeffrey. I mean, 207 00:10:06.519 --> 00:10:09.080 the view that code reviews are about finding defects is 208 00:10:09.919 --> 00:10:12.879 primarily about finding defectses is a really widely held one. 209 00:10:13.519 --> 00:10:15.360 One of the things I mentioned in the article is 210 00:10:15.360 --> 00:10:19.000 there's a really interesting piece of academic research that we've 211 00:10:19.000 --> 00:10:22.720 done at Microsoft in I think twenty thirteen, where they 212 00:10:22.840 --> 00:10:27.639 examined hundreds of code reviews across large parts of their 213 00:10:27.679 --> 00:10:30.480 code base, and they analyzed each code of view in 214 00:10:30.559 --> 00:10:33.360 terms of what the actual outcome of that code review was. 215 00:10:33.399 --> 00:10:36.440 They looked at every comment on the poor request and 216 00:10:36.480 --> 00:10:40.559 they categorized them all, and they basically figured out that 217 00:10:40.600 --> 00:10:43.120 the most common outcome of code reviews was basically people 218 00:10:43.159 --> 00:10:46.200 asking questions about something, other people explaining things, and that 219 00:10:46.240 --> 00:10:49.879 process of communication, learning about the code or discussing the 220 00:10:49.919 --> 00:10:53.879 code is the most common outcome. Whereas when they interviewed 221 00:10:53.879 --> 00:10:55.440 the developers and said what do you think is the 222 00:10:55.440 --> 00:10:57.879 most common outcome of code reviews, they said, oh, it's 223 00:10:57.879 --> 00:11:00.360 all about finding defects. But there was a just found 224 00:11:00.360 --> 00:11:02.559 The funny effects is it does happen. Of course, it 225 00:11:02.600 --> 00:11:04.960 does happen. Sometimes people will find a bug in code 226 00:11:05.039 --> 00:11:07.559 just from looking at it visually, but there's actually a 227 00:11:07.559 --> 00:11:10.879 lot of other stuff going on. There's there's coaching and mentoring, 228 00:11:11.279 --> 00:11:13.600 there's you know, exchanging ideas and approaches. 229 00:11:13.639 --> 00:11:14.440 There's not all of it. 230 00:11:14.440 --> 00:11:16.600 It's just about saying, hey, we found a bug, you 231 00:11:16.720 --> 00:11:18.879 got this wrong, this needs to change, and although you 232 00:11:18.919 --> 00:11:20.480 know that is one possible outcome. 233 00:11:20.679 --> 00:11:23.720 Yeah, I like that in the sense that how do 234 00:11:23.759 --> 00:11:26.600 I put it? Because when we do the code reviews, 235 00:11:26.919 --> 00:11:29.559 a lot of the communication in the code reviews is 236 00:11:29.600 --> 00:11:32.080 about that, right, I didn't see anything wrong with this, 237 00:11:32.600 --> 00:11:34.960 and so I approved the pull request. That's kind of 238 00:11:34.960 --> 00:11:37.399 how we run at work. I mean, we do mob programming, 239 00:11:37.960 --> 00:11:40.600 so you know, we have different teams of like three 240 00:11:40.720 --> 00:11:43.519 four people sometimes two people, you know, and we just 241 00:11:43.799 --> 00:11:46.840 get on a remote call. It's exhausting, by the way, 242 00:11:47.120 --> 00:11:49.240 and then we just we code all day. But then 243 00:11:49.480 --> 00:11:52.120 we pass off our work in a PR to somebody 244 00:11:52.120 --> 00:11:55.080 else who does a code review on it. And yeah, 245 00:11:55.159 --> 00:11:57.159 a lot of it is just we want somebody to 246 00:11:57.200 --> 00:11:59.279 look at it and give feedback on it. 247 00:11:59.320 --> 00:12:00.759 But it's not that specific. 248 00:12:01.279 --> 00:12:03.720 And I think that's something that I'm kind of pulling 249 00:12:03.759 --> 00:12:05.440 out of this, out of your article and out of 250 00:12:05.440 --> 00:12:08.480 what Jeffrey is pointing out, and what you're saying is that, 251 00:12:08.919 --> 00:12:12.039 you know, we've got to basically open the door and say, hey, look, 252 00:12:12.039 --> 00:12:15.080 it's okay for you to say, hey, I don't understand this. 253 00:12:15.200 --> 00:12:17.720 Can you train me on it? Right? Or I don't know? 254 00:12:17.799 --> 00:12:20.399 I you know, I don't understand this approach. Can you 255 00:12:20.440 --> 00:12:22.120 explain to me why we went this way in not 256 00:12:22.159 --> 00:12:25.879 a different way? Or I'm trying to understand this particular 257 00:12:25.960 --> 00:12:29.039 area of code, you know, or maybe I do see 258 00:12:29.039 --> 00:12:32.559 that this thing could be problematic, either for maintenance or security, 259 00:12:32.759 --> 00:12:34.960 or you know, any of the other number of things 260 00:12:35.000 --> 00:12:37.279 that we wind up coming back around to because they 261 00:12:37.360 --> 00:12:40.159 bid us in the rear end at some point, right, 262 00:12:40.519 --> 00:12:42.799 and you know, in those kind of fall toward defects. 263 00:12:43.120 --> 00:12:46.320 But some of those are really kind of how do 264 00:12:46.399 --> 00:12:48.759 I put it, they're they're kind of cloaked to something else, 265 00:12:48.840 --> 00:12:51.159 you know, if it's a maintenance issue, and then we 266 00:12:51.240 --> 00:12:54.000 eventually clean up what's causing the maintenance issue, it's not 267 00:12:54.000 --> 00:12:56.639 necessarily a defect. It is a defect, but we don't 268 00:12:56.639 --> 00:12:57.840 think of it in the same way as we think 269 00:12:57.879 --> 00:13:00.759 of a bug, right because the code still works. It's 270 00:13:00.840 --> 00:13:03.159 just every time we go change it. We have to 271 00:13:04.080 --> 00:13:05.440 you know, jump through a whole bunch of hoops to 272 00:13:05.440 --> 00:13:07.679 make sure we didn't break anything. It's a maintenance nightmare. 273 00:13:08.360 --> 00:13:10.759 And so you know, and I can kind of imagine 274 00:13:10.759 --> 00:13:13.639 the same thing with you know, whether it's infrastructure or 275 00:13:13.759 --> 00:13:17.000 chef recipes or you know, whatever you're dealing with, that 276 00:13:17.120 --> 00:13:19.120 same kind of thing. And so yeah, if somebody can 277 00:13:19.120 --> 00:13:21.639 pass along, hey, look we do it this way because 278 00:13:21.679 --> 00:13:25.200 we found that it saves us all these other hassles 279 00:13:25.200 --> 00:13:28.320 in the back end. And what's funny is is sometimes 280 00:13:28.559 --> 00:13:30.639 you'll get the new person in my I have a 281 00:13:30.639 --> 00:13:33.120 fifteen year old who's playing video games now, so the 282 00:13:33.200 --> 00:13:36.679 total nube will come in, right, and they'll say, oh, well, 283 00:13:36.679 --> 00:13:39.799 why don't you try this other approach then? Right, that 284 00:13:39.879 --> 00:13:42.039 has all those benefits you just listed for why you're 285 00:13:42.039 --> 00:13:45.000 doing it this way, but also gives us this other 286 00:13:45.039 --> 00:13:47.039 benefit that I'm seeing we might be able to add. 287 00:13:47.559 --> 00:13:49.759 And then all of a sudden, you have a better process, right, 288 00:13:50.279 --> 00:13:54.679 And by understanding and documenting and pushing and communicating, that 289 00:13:54.919 --> 00:13:58.039 opens up all these doors. And that's really yeah, that's 290 00:13:58.039 --> 00:14:00.519 what we're talking about with these code views. And that's 291 00:14:00.879 --> 00:14:03.320 back to the original idea. It's all of the above. 292 00:14:03.960 --> 00:14:06.679 It sounds like you have a process, you know, after 293 00:14:07.240 --> 00:14:09.919 reading through this article, you have a process for conducting 294 00:14:09.919 --> 00:14:12.320 these code reviews that make sure that all these things 295 00:14:12.320 --> 00:14:13.480 get communicated about. 296 00:14:13.559 --> 00:14:13.720 Right. 297 00:14:13.799 --> 00:14:16.159 It's not just opening the door for it, it's explicitly 298 00:14:16.159 --> 00:14:19.360 calling out, hey, are we thinking about security? Are we 299 00:14:19.399 --> 00:14:22.720 thinking about code structure? Are we thinking about you know, 300 00:14:22.840 --> 00:14:26.399 these different levels of concern so that at the end 301 00:14:26.440 --> 00:14:28.759 of the day we know that we've covered our bases 302 00:14:28.799 --> 00:14:30.679 here and we're communicating about the things that we care 303 00:14:30.720 --> 00:14:31.519 about in our code. 304 00:14:31.679 --> 00:14:31.879 Yeah. 305 00:14:31.919 --> 00:14:34.360 Absolutely, I mean I think you've hit on an interesting 306 00:14:34.399 --> 00:14:36.440 point there, which is about who does code reviews or 307 00:14:36.480 --> 00:14:39.360 who can do a code review. We were quite clear 308 00:14:39.519 --> 00:14:41.639 in the discussions we had at the BBC that we 309 00:14:41.720 --> 00:14:45.039 thought that developers, engineers of all levels should participate in 310 00:14:45.080 --> 00:14:47.879 the code view process. It shouldn't be a situation where 311 00:14:48.200 --> 00:14:50.360 my code can only be reviewed by somebody who is, 312 00:14:50.960 --> 00:14:53.200 you know, a pair of mine, or somebody with more 313 00:14:53.240 --> 00:14:55.799 experience than me, or more familiarity with this than me. 314 00:14:56.600 --> 00:14:58.919 Just for the reasons you've given that that quite often 315 00:14:58.960 --> 00:15:02.320 you get really interesting perspectives and questions and challengers from 316 00:15:02.440 --> 00:15:06.080 people who are new to develop new to engineering, new 317 00:15:06.120 --> 00:15:10.000 to this particular code base, and so in order to 318 00:15:10.039 --> 00:15:13.200 try and help facilitate that, we put some some prompts 319 00:15:13.200 --> 00:15:16.039 and suggestions into our code reviews guide, where we're saying, 320 00:15:16.240 --> 00:15:18.000 if you're a new developer, if you're need to this 321 00:15:18.039 --> 00:15:20.120 code base, here are some things you can try when 322 00:15:20.159 --> 00:15:21.799 you're doing a code review. Here are some things you 323 00:15:21.840 --> 00:15:24.759 can ask Have you spotted something that looks a little 324 00:15:24.759 --> 00:15:26.720 bit inconsistent? I mean you could ask the author to 325 00:15:26.759 --> 00:15:29.519 explain that. Have you asked the author about if they've 326 00:15:29.519 --> 00:15:34.440 considered security, performance, accessibility, other kinds of non functional requirements. 327 00:15:34.440 --> 00:15:36.840 So a bit of a cheat sheet almost where you 328 00:15:36.840 --> 00:15:38.799 can say, you know, these are things we should be 329 00:15:38.840 --> 00:15:41.559 thinking about when we did code reviews. And it brings 330 00:15:41.679 --> 00:15:44.279 more people in the team into that process, right. It 331 00:15:44.360 --> 00:15:47.840 means that more junior engineers or less experienced engineers can 332 00:15:47.879 --> 00:15:51.759 participate and actually give just as much value to that 333 00:15:51.840 --> 00:15:54.799 process as people who you know, have been working on 334 00:15:54.840 --> 00:15:55.600 the code for years. 335 00:15:56.080 --> 00:16:00.840 What James is, when you're talking about kind of you know, 336 00:16:00.919 --> 00:16:04.759 having junior developers doing code reviews and these kind of things, 337 00:16:05.120 --> 00:16:07.200 it seems like a large part of what you're doing 338 00:16:07.279 --> 00:16:10.960 is kind of reframing the intent of the code review. 339 00:16:11.159 --> 00:16:12.559 And I don't know if this is a problem you 340 00:16:12.600 --> 00:16:16.399 guys have had the BBC, but I've seen teams in 341 00:16:16.440 --> 00:16:20.200 the past where junior developers very intimidated by the code 342 00:16:20.200 --> 00:16:22.879 review process, and you know, it's kind of seen as 343 00:16:22.919 --> 00:16:26.200 a gatekeeping role where they're checking in code and then 344 00:16:26.480 --> 00:16:29.039 senior developers are looking at it and kind of you know, 345 00:16:29.600 --> 00:16:32.519 going over it. So I really like the kind of 346 00:16:32.519 --> 00:16:34.919 idea of reframing it so that it's you know, code 347 00:16:34.919 --> 00:16:39.320 reviews are everyone's responsibility. It's a collaborative team process, and 348 00:16:39.360 --> 00:16:43.200 it's more about the communication rather than the gatekeeping, even 349 00:16:43.200 --> 00:16:45.519 though you still get maybe some of those gatekeeping functions 350 00:16:45.519 --> 00:16:48.759 if you do need to have security reviews or have 351 00:16:48.879 --> 00:16:52.799 senior developers reviewing code and looking over those things. Just 352 00:16:52.840 --> 00:16:55.440 by reframing it you get all the benefits and maybe 353 00:16:55.600 --> 00:16:59.480 a much better participation rate, particularly in a remote context 354 00:16:59.559 --> 00:17:03.399 where onboarding developers can be a little bit more intimidating, 355 00:17:03.480 --> 00:17:06.200 Especially for junior developers. You don't have that kind of 356 00:17:06.240 --> 00:17:09.160 face time. You know, it's a little hard build team 357 00:17:09.160 --> 00:17:12.319 cohesion remotely in those kind of things. Is that the 358 00:17:12.400 --> 00:17:15.680 correct kind of analysis of the direction you're going with. 359 00:17:15.599 --> 00:17:17.920 This, Yeah, absolutely, Caleb. 360 00:17:18.039 --> 00:17:21.839 We found that, particularly we're going more remote first, that 361 00:17:22.519 --> 00:17:25.920 junior developers, less experienced developers were quite wary of the 362 00:17:25.960 --> 00:17:29.000 process and quite reluctant to get involved. And you know, 363 00:17:29.279 --> 00:17:31.000 as we were saying earlier, you can't just lean over 364 00:17:31.039 --> 00:17:32.839 to the person sat next to you at your desk 365 00:17:32.920 --> 00:17:35.640 and you know, informally ask for help. Everything is a 366 00:17:35.640 --> 00:17:37.960 bit more formal, a bit more structured once it's done 367 00:17:38.519 --> 00:17:43.200 just via pull requests. We wanted to really encourage people 368 00:17:43.680 --> 00:17:46.319 at old levels to get involved in that process. And yeah, 369 00:17:46.519 --> 00:17:49.039 I think we were really reframing code of views a 370 00:17:49.079 --> 00:17:52.200 little bit and make it feel making it seem more 371 00:17:52.200 --> 00:17:57.119 like a collaborative process that is benefiting everybody in the team. 372 00:17:57.680 --> 00:17:59.200 I mean, I think we have to recognize as well 373 00:17:59.240 --> 00:18:01.640 that code views can be quite demanding, can be quite 374 00:18:01.640 --> 00:18:04.960 stressful in fact, for people of all levels of experience. 375 00:18:05.000 --> 00:18:07.359 You know, having your work looked at by other people 376 00:18:07.480 --> 00:18:10.599 can be challenging sometimes, But at the same time, it's 377 00:18:10.640 --> 00:18:13.039 part of being a software engineer that you people are 378 00:18:13.039 --> 00:18:14.279 to look at your code and you have to be 379 00:18:14.319 --> 00:18:17.400 able to receive feedback about your code and give feedback 380 00:18:17.440 --> 00:18:20.759 on other's code and get comfortable with that process. So 381 00:18:21.640 --> 00:18:25.200 there is obviously still a gatekeeping function within code reviews, 382 00:18:25.359 --> 00:18:28.480 and there's an important role for that, but we wanted 383 00:18:28.519 --> 00:18:31.200 to try and promote the idea that the team owns 384 00:18:31.240 --> 00:18:35.240 the code collectively. It's not like up to an individual 385 00:18:35.279 --> 00:18:37.319 person to decide whether something is okay or not. 386 00:18:37.359 --> 00:18:38.720 It should be a collective decision. 387 00:18:39.160 --> 00:18:41.799 People can have different perspectives on that, and to make 388 00:18:41.839 --> 00:18:44.240 it a bit more of an approachable process that it's 389 00:18:44.279 --> 00:18:47.039 not so seen as in a really intimidating thing where 390 00:18:47.480 --> 00:18:50.759 this really senior experienced person will come and give a 391 00:18:50.839 --> 00:18:53.480 verdict on your code. It's more of a dialogue where 392 00:18:53.519 --> 00:18:55.359 they say, okay, so why do you approach the problem 393 00:18:55.400 --> 00:18:55.839 this way? 394 00:18:56.119 --> 00:18:57.359 Have you considered it this way? 395 00:18:57.759 --> 00:18:59.960 It's not just, you know, just the kind of pure 396 00:19:00.079 --> 00:19:03.160 gatekeeping process that it might have been seen as occasionally 397 00:19:03.160 --> 00:19:03.720 in the past. 398 00:19:04.079 --> 00:19:06.359 Yeah, I'm going to step into my confessional here for 399 00:19:06.400 --> 00:19:09.039 a second, because so the job I'm in now when 400 00:19:09.079 --> 00:19:11.799 I got hired, so my full time job, I'm actually 401 00:19:11.799 --> 00:19:14.799