Isaac: Welcome to Never Rewrites. I'm Isaac Askew. Dustin Rea: Justin, right? Jeffrey Sherman: And I'm Jeffrey Sherman, and today we're gonna discuss AI-driven p pull requests and how they're getting too large, or are they too large? And of course, it depends is always the answer. So Dustin Rea: Mm-hmm Jeffrey Sherman: we're good here. See you next time, yeah. Isaac: See you next time for this week. Dustin Rea: Hahaha Jeffrey Sherman: I I brought this topic up because I was years ago, and by years ago I mean like decades, plural. I was the a-hole who would have these giant PRs because as I was working on a project or a a ticket, I would fix things as I went along. I could do linting changes and I would fix this and I would fix that. And people would get this thing, it was to fix a bug, and there would be twenty other classes that got changed, had nothing to do with it, and everyone hated me or everyone hated doing my PRs because it added such a cognitive load. And I have since reformed my ways and repented and become much more of the your PR should be as small as possible, if not smaller. And with AI, we are now going it things are swinging the other way, where AI will just d you don't need to break things down because it doesn't have the cognitive overload to deal with all these things. And you get these giant PRs that no human could possibly review. in a reasonable way, right? Like it's the the old joke, if you have a twenty line PR you'll get feedback. If you have a thousand line PR, somebody will say, Looks good to me. Yeah. Yeah it's fatigue. Like Isaac: that's so true too. It still happens to me. It's just fatigue, yeah. Dustin Rea: Mm-hmm. Mm-hmm. Yeah. Jeffrey Sherman: you you won't do it. It's it you're human. And so is this a bad thing? And I say probably most of the time yes. That if you've got a 5,000, 10,000, 20,000 line PR, that is Isaac: Mm-hmm. Jeffrey Sherman: probably too large of it. The AI, you in the AI have taken too large of a step. And you probably should have broken that down into multiple smaller PRs. I one just because the of the cognitive load, and two, the larger the thing is, the more. it likely that it's gone off the rails somewhere and you haven't noticed. But I think Dustin, you kind of are swinging the opposite way of well, you know, if it's a new feature and it du you know, and it's written all the tests, right, I I think that's a good caveat in this case. It's right, Dustin Rea: Yeah. Yeah. Jeffrey Sherman: we're talking about good code or theoretically good code. Dustin Rea: Yeah, we're assuming. Assuming good code. Isaac: Mm-hmm. Dustin Rea: Yeah, I think. Let's jump back and talk a little bit about like, you know, what is what is the purpose of code review, right? The purpose of code review is to catch, you know, issues with the code or maybe there's logical fallacies or like there's some contradiction or, you know, maybe there's an interface mismatch or a very common one, which I shared a plugin before this about, like this exists already. We have this somewhere else in the code, which AI is notorious for not referencing or using what's already there and just building completely new stuff, Isaac: Mm. Dustin Rea: right? So if you're looking at a... Jeffrey Sherman: I I've had a rebuild f test unit test frameworks on me. I'm like, what this is part of the unit testing framework. Dustin Rea: Yeah Isaac: Cheese. Dustin Rea: Yeah, yeah, they're generated I said this last time it generates I have a generate logos and it's like why are you generating logos? We have a logo. There's no need to generate a new look So yeah, I think you know in in that case you want to be able to catch those things In review, but like if you think about it, how many of those things? need a human to verify them so at the same time maybe we were, we could only handle certain size PRs with the tools that we had 12, 24 months ago. But now the same way that we've adapted our coding practices, I think it's now time to adapt our review practices in the same way. I don't think that that means by default, big PRs, you know, are of value, but I guess the flip side to the argument is, is how do you... continue with the velocity without just holding everything in review. at that point, what is the value Isaac: Mm. Dustin Rea: of human review? If humans aren't writing the next set of changes on top of the changes that you're reviewing, you really only have to understand it at the system level anyway, because you're not actually going to be the one connecting the lines of code anyway. Jeffrey Sherman: Sure, right and Dustin Rea: So then I will almost say that the argument starts to lean towards is human legibility even as valuable as it was 12 months ago? Jeffrey Sherman: It is not. there's Kent Beck, one of the original extreme agile programmers. he's he was talking he's got a podcast now and he was talking about how he's written books on how to make clean code and code that's good for human developer for human human readable code and code that's clean and elegant. And he's like, This book no l is no longer relevant. Isaac: Yeah. Dustin Rea: So is it just a matter of time before there's even maybe a new programming language where maybe humans don't even understand it? I think that gets way far into the future, but I think that's kind of like where it's shifting from us, right? if, what do we? Isaac: There's actually an AI the the AI fear people, I think neural ease is the the language that they're the that they invented that they're talking in or something where it's like Dustin Rea: Yeah, yeah, yeah, Isaac: it doesn't use English like as AIs talk to each other because they can talk to each other faster than using English. So I imagine that would be the case. But this kind of reminds me of a previous concept where where we were talking about using some standards in your code is still nice because if you if you do something where like if you mark paid true and that actually doesn't mean paid. Because you did something really weird, which we did in a previous thing, that will still confuse the AI sometimes. So it's not necessarily that things need to be human readable and perfect, but they still should be s I think I think clean code is still somewhat readable in that fashion, that things kinda still make sense to a human, even if something something could read it faster. If you Dustin Rea: Mm-hmm. Isaac: you know, like before it was like all the camel case names or people who had notoriously long function. Names that you know you've Jeffrey Sherman: Look, I learned with reverse Polish notation and I liked it. Dustin Rea: All right. Isaac: Well, usually if it was a really long function name, that just meant your function was too complicated and you could break it apart and then you still have it readable, but it's Dustin Rea: doing too many things. Isaac: yeah. But then it's across many files. but yeah, I think that's a really good point, Dustin, like with the idea of like who is the re who is the review for? And if it's Dustin Rea: Right. Isaac: if the bottleneck now if you're if AI is generating so much code that the bottleneck becomes review and then you get fatigue from review, is there a way to eliminate that review such that We don't need as many humans reading it or bigger MRs and AI helps with the review itself. You know, where does the human get re removed from the loop? Dustin Rea: Yeah, I think one thing that I've seen I've changed my review practices a little bit to think about less in is it readable by a human versus is it kind of human understand it? Like, is it coherent? Like, is this is it constructed in a way that it somewhat matches how if I sat engineers in a room and came up with an ADR and like did all the stuff, would this be coherent? Like, would this be a coherent system? And I think if you use that framework, you'll often find the places where it isn't coherent, like just as same as when it's humans and we just miss something, there's what we call gaps or whatever, we just missed that this system Isaac: Mm-hmm. Dustin Rea: also needed to pull that data in. So I kind of use that framing instead of like, you how clean is the code? I think the code needs to be clean enough to read the coherence of the code. So if you have crazy function names or you're not following like solid, like you're not using single responsibility principle, I still think clean code applies, but for a different reason. So it's readability not from the sense of because I'm gonna be there at 2 a.m. responding to an incident or because I'm the next person that's gonna be building the feature, but it's because I need some way to verify that what the code that the AI has written is coherent with the request. Jeffrey Sherman: So this is an interesting I I think you you you brought up a really good point, Dustin. And I think what's interesting is in my mind, the size of the PR was an implicit way of limiting the scope, the size of the change before you showed it to the user. Right? And so it wasn't it's not that the PR needs to be small, it's that the unit of work that you're giving before you check in with the customer should be small. And PR size was the I guess the h the historic like the you do a PR, you show you release to production, you show it to the customer, boom, that's the cycle. Then you go back, get the feedback, and you come back, and now with the A AI it's like, well, sure, I can do a 20,000 line change. It took five minutes, and now I can go show that to the customer. The those two things you the the PR size and the customer impact change used to be tightly coupled. And what you're saying, Dustin, is those are things are no now no longer coupled. Dustin Rea: Yeah, I don't want to say that they're not coupled at all, but I would say it like this. So like, let's even, let's use another, let's take AI out of it and just see, I bet I can build the same case without AI. So like, let's say you've got a principal engineer on one hand and a junior engineer on another hand. If both of these people submit 20,000 line MRs. I'm probably going to actually read the one from the principal engineer, but by default reject the one from the junior. Cause there's just no way, right? Like the, like the reality of that is probably not there. Isaac: Reality of that. Dustin Rea: Like there's, need to like sit down and like rethink about it, right? Like I'm not even going to look into it because it's going to be, you'll open it up and there's spaghetti code. And it's like, okay, already we know this isn't going to be coherent. Like the system's kind of all mixed and matched together. Like that's not going to work. so I think it's not necessarily about the size, the size only in relation to what you can realistically manage. If you can realistically manage a 20,000 line review because you're not reviewing it in the same level of scrutiny that you would if it was a human, because a lot of your automated checks are taking care of some of the stuff that you manually had to look for before. And I think, I think Isaac was saying this before we got on, you know, I think this has forced a lot of teams to improve their developer experience, even though the developers aren't the ones experiencing it because the AI has no judgment. So like it has to be like a hundred percent correct all the time. It can't be like, well, if you do it this way, you kind of have to know about this thing that's not documented anywhere. And like that team knows about it, but no one else does. Like that doesn't work in AI. So it's like, have to remove these things now. So then that doesn't need to be reviewed Isaac: Mm. Dustin Rea: for because it's no longer a hidden risk. Isaac: Yeah, I think f for me that or that what you're referencing was just a you know a project I'm working on. But that evolved for me, out of a fear of am I what if I make this big PR that actually doesn't do anything? You know, like I had Claude generate it, then I look stupid, right? 'Cause Dustin Rea: Yeah, incoherent. Yeah. Yeah. Isaac: someone like look looks at the code and is like, What are you even doing? Does this even solve the problem? And I'm like, Well, Claude did it nice. I'm just gonna throw it on staging and see if it works. You know, so I'm like, Okay, Dustin Rea: Yeah. Yeah. Isaac: well that's that's a dumb way to do it. So the The better way to do it is make sure my local environment is as close to parity as possible to staging and hence production. And then run a whole suite locally and then prove all the data matches what it was expected to, then all the screenshots of any front end flows were corrected to to match the Figma designs. And then once I can prove, okay, locally, here's my screenshots, I'll attach that to the description of the the PR. This looks pretty good, the flow looks good. I spot checked everything. It's just essentially just checking your own work. But it is checking Claude's homework before I insult somebody with a with a request for a code review that doesn't do what it's supposed to do. That's how that evolved Dustin Rea: Yeah. Yeah. Isaac: for me. And I think that's for me that's been a really good way to to to to check it instead of just like, you know, hitting enter on a prompt and going to lunch and being like, Hey, can you review a code for me, man? Which is kinda kind of a Dustin Rea: Yeah. Isaac: messed up thing to do if your code actually doesn't do it and someone gets mad reviewing all that code. Jeffrey Sherman: Yeah, old person story, I but Isaac: Yeah. Jeffrey Sherman: you know, twenty years ago I have definitely been asked to you know, and more recently too, but like I've been asked to review code. I'm like, your your code didn't compile. Dustin Rea: Yeah. Yeah. So this goes back to standards, right? So Isaac: Exactly. Dustin Rea: you shouldn't be reviewing code if the checks don't pass. you don't have checks? Well, then there's kind of like your first problem. You're using humans to that's a very expensive PR checker for like, used to say this when I, won't mention the person, but there was a person basically doing linting in code review, like human, like when they had no Isaac: yeah. Dustin Rea: linter. So like, instead of taking the time to set up the linter and make it automatic for everyone forever, for, know, it was just doing it manually. So it's like, it's a very expensive linting machine. And I think the same case can be made about Isaac: Same case now. Dustin Rea: like a lot of the AI. stuff like, you know, can you get a better output from the beginning because of, you know, if you've done the architecture work upfront, you can kind of give it that higher level guidance. And then you're just basically checking, did it, did it, how well did it adhere to the standards that I gave it? You and then you correct all of that before you push it off on another human. But it's the same problem as before, right? Like if even before AI, if you were the type of person to write a PR, not check it locally, not wait for checks, not run your tests, not see if you met coverage. Isaac: Right. Dustin Rea: Like you could sell someone to go, you know, review your PR, realize you didn't meet coverage, and now you've got to make more changes and you need another review, you know. The same thing I think could be happen. Isaac: It's just a waste of people's time. Dustin Rea: Yeah. Isaac: Yeah. I just call it disrespectful at that point. I'm like, would be w would I appreciate someone hit me with that review and I'm like, it doesn't compile, Dustin Rea: Yeah. Mm Right. Isaac: are you kidding me? Like, at least have it work first, you know. Jeffrey Sherman: Right. Did did you actually like run this? Like your unit test failed. Isaac: And no, yeah. If they didn't, then like what are you doing here? Like what what title are you at? What what Dustin Rea: Yeah, yeah. Isaac: level of engineer are you? Like, come on, man. Dustin Rea: Yeah, I think the testing that you were hitting on to Isaac has been, it has been pretty critical on my flow. I think I I've seen a lot, especially in the startup phases and I don't mean just self funded, but even in the VC funded, like you'll see often people skip end end tests just because of the expense. Like it just takes developer time to write. They're a pain to maintain. They can be kind of flaky, you know, so there was a lot of argument in the past about what was the right amount of like end to end tests to like get the right validation. And I think now we can even lean harder on those where it's like if it comes Isaac: Yeah. Dustin Rea: through with a demo and none of the other tests break that and they all have the same level of scrutiny because they've been built in this way and you can you can continue to keep building on top of this but I still don't think that that removes Jeffrey's argument that there's a risk factor based on the size or complexity of the change and I don't think that's something we've really like dove into in this this talk like how do you how do you actually make a difference there. Jeffrey Sherman: Right, and I think c there was an implicit thing that we were talking about before the show started of a new feature, right, new development, a large PR is less risky than in a existing code. If you're changing something and the change is giant, that is implicitly more risky than if I'm writing something new and it's giant. Dustin Rea: Yeah. Isaac: Mm. Maybe. My my my first thought is like, what if you have a really bad query in your brand new thing that can tank the database and you didn't think about that? Like you could still write something really risky against that doesn't scale well and yeah and and hurts your legacy thing. maybe maybe maybe like the argument of it being less risky is still less, but it's still it's still there. That's something my end to end test doesn't check, right? I have to actually go through and manually spot check the query if it wrote a new query to make sure it's a scaling query. So I I haven't solved that one. Yeah. Dustin Rea: I was even thinking of like, I was thinking of like usage when Jeffrey said that I was thinking of like, okay, if it's Isaac: Mm-hmm. Dustin Rea: a new feature, no one has any expectations about it yet. If it's a feature that they Isaac: Mm. Dustin Rea: use every day, and then you break it, that is a big like, it's an incident, like, that's an immediate incident, you know, that's on you. And I think maybe you have a different size relative change to those risks then maybe a large change PR is like you said in that five to 10 K range, but a very large new features like, it's like 10 to 20. It's like the range gets bigger, you know, cause you're putting every, it's a whole new module. So like, you gotta think, you know, maybe some of that's boilerplate it's, you know, tests it's documentation it's, know, so I think the five to 10 K actual code lines change is probably where it starts to be the threshold of like what a human can manage day in day out every single time. Isaac: One side thing to think about too. that I I've noticed that Claw does a good job of that humans didn't in the past when it comes to fifty thousand lines of code. is commits. Claw does a pretty good Dustin Rea: Mm-hmm. Isaac: job at like as it's thinking through, making commits along the way of each change. So it's not just one commit with everything. It's how that thing changed over time, which is really good for itself as it goes back through to see how the component changed. And it'll flag like the the st the the Jira ticket and you the story, everything about that. That way you can go back through and go, this is how that sing this thing has evolved. The human won't do that. Well, maybe they'll do it with like few like three commits instead of like thirty, you know, Dustin Rea: Mm-hmm. Isaac: or just like delivered the feature and then it's fifty thousand lines, Dustin Rea: Yeah. Isaac: right? When you can't really tell how things evolved. And that's a kind of a risky thing there too. So I Claude does it l i at least if it's risky, it's a slightly de-risked or you can see a paper trail with the commit history, which makes it better. Dustin Rea: You still squash and merge, right? So you only have it during the... you're putting the whole thing in. Okay. Isaac: No, I leave all the commits in there. Every every little change it makes. I think I think commit history is incredibly valuable. And especially for like going back through and understanding what the original developer's intent was, I can see over a Dustin Rea: Yeah. Isaac: ten year history why this thing changed. Dustin Rea: Yeah, yeah. Isaac: And that would take me forever as a human to understand. All the different pieces across different repos. And so commit history is really valuable for me. Dustin Rea: yeah, yeah, platform mapping too, yeah. Jeffrey Sherman: Yeah, that's an interesting point. yeah, I've always been slightly against squashing, but it it just seems like that's what everyone does, so but that's not a Isaac: I used to do it all the time. Like I used to squash 'cause I thought it was cleaner. And that you know, I Dustin Rea: I still think squashing is cleaner. think I might die on that hill. Because I think maybe I'm the type of developer that it makes more sense, because I'll thrash. Isaac: That's fine. Dustin Rea: But I want to thrash. I want to be able to thrash and not mess up the history. So I may thrash on my local machine and still commit those. But then I don't want that to be in the official history of like, it's just experimentation. It's not the real, it's not how it came out. Isaac: Maybe maybe it depends. Dustin Rea: Yeah, yeah, I'm almost interested to like think about that more because I'm like, man, I've been I've been really hard on like squash and merge for like years, you know, because I think that. Yeah. Isaac: I have too, until AI. Until I saw how well it pieced together. Like I can look at Git history and see how something changed over time for one file, but how that's orchestrated across every other file and multiple repos, which you can do. I just can't do it at the at the level as a human. And once I saw it pull Jeffrey Sherman: Well, we're running Isaac: in all those commits, I was like, ooh, this is really useful. Jeffrey Sherman: Yeah, we're running a little long, but I think that would be maybe next time of does AI change what you you know, that you would wanna not squash because the history Well because Dustin Rea: branching rules. Jeffrey Sherman: one, the it's the AI isn't gonna thrash the same way, although it does thrash. and two, Dustin Rea: Yeah, right. Jeffrey Sherman: AI is actually capable of looking at that giant history and and making something of it where humans aren't. Dustin Rea: Right, yeah, humans like, okay, this was the change. Like I always think of it as like, I want to be able to cut back to a certain point, you know. Isaac: Yeah. Maybe we should dive into that deeper and see if we can convince Dustin to not die on that hill. Dustin Rea: Yeah, yeah, I think that'd be a good one. Jeffrey Sherman: Yes. Alright, but I think we have d did we come to any conclusion about I think we left it as it depends. should PRS be Dustin Rea: I think this was the point. Yeah, we ended up where we started. Jeffrey Sherman: should PRS be smaller in the age of AI? Well, it depends. Isaac: That's not satisf a satisfying answer for me. We're gonna keep diving into this one. There'll be a part two coming soon. We need a good answer. All right. Jeffrey Sherman: No, it's not satisfying at all. Yeah. All right. Thank you Dustin Rea: Part two coming soon. Jeffrey Sherman: all for listening. I'm Jeffrey Sherman. Dustin Rea: Dustin Ray. Isaac: And I'm Isaac Askew, and this is Never Rewrite.