Welcome to No Compromises. A peek into the mind of two old web devs who have seen some things. This is Joel.

And this is Aaron.

Every time I open a pull request and send it to you, Aaron. It's like a little gift that I'm giving you, and I could just... Putting myself in your shoes, you get that email and you're like, "Oh, yeah, this is the highlight of my day." Is that a pretty accurate assumption of how you receive those?

Exactly. When I see a pull request, I'm like, "Oh yeah, it's time for code review." Yes.

And even better, what if there's like 10 of them and some of them are really big? You love that.

Well, I'll tell you what. It happens, there's a lot of times when we get that really big one. It just depends on who did the development and how they review it. But one of the things that you do, I've noticed, to make code reviews easier is you'll go and make a bunch of changes, but then you'll do atomic commits. You'll say, "This is a commit that encompasses all the functionality for X, Y, Z," and then you move on and you move on. And I know that because you do that yourself, you also sort of leverage that when you do code reviews, right?

I do sometimes. I don't always, it does depend on the size of the pull request. I want to give a tangible example because there's a couple of different types of pull requests you get from me. I think our workflow might be a little unique. Obviously, there's the code, right? We are working on a feature, on a project, you review it. But with our educational work, we have the daily newsletter, we have a book on validation. And those go through content review as well, which involves pull requests. I just did a fairly hefty revision to one of those books. There was tests that go with the content, there was code, but there was also pros, right? Like writing, describing how validation works, and code samples within that. The reason I bring that up is because when I get a pull request, I'm thinking mainly in code, and that's where I think atomic commits help. But I did also use atomic commits on the content, updated this particular rule, updated this for Laravel 12, whatever, to kind of describe it. But I know you don't look at it that way so is it even still useful for me to do that? To see the commits, even if you don't review it commit by commit?

Well, I think there's a couple of questions that... we kind of muddied the water when you talked about, "Well, this is for Laravel 12," or whatever. We want to have good commit messages, we want to bring them forward and we want to have every commit makes sense in the context of itself. So that's not necessarily about packaging stuff. When I talked about those larger commits, I think what... Maybe you did this, maybe you didn't, doesn't matter. But you maybe had your branch and you branched off of that, did six or seven little things committing along the time, and squash merged it back into the main branch, the feature branch, and said, "This is a commit for X, Y, Z feature." You kind of mentioned that that's kind of all you do for larger things. It sounds like you were doing that maybe for the reviewer. How would you make use of that, though?

This is where I find it useful as the reviewer. You're questioning me about my review, and I'm question you about your review, but we're going to get back to you, Aaron. For me, when I look at a pull request that's a little larger, and especially if within the pull request... Let's just say it's a feature, we also bumped a version of PHP unit, right? Like a bunch of tests got updated, and it's mechanical changes. Instead of the data providers being an annotation now they're attributes, or what whatever it is. If there's a bunch of files touched and within there there's a feature change then that's when I like to look at the commits because I can isolate it. Like, this one commit might have 300 changed files but it's all the same thing, so now I can scan that and my eye is sort of looking for those patterns. And I'm only looking for the things that kind of jump out like, "Whoa. Why is that different than the other one? Why is there two blank lines after that instead of one?" You know, whatever it is. But generally speaking, the feature I do review holistically as a PR. File by file, not commit by commit. Does that explain it? My decision for when I switch to the commit by commit review mode?

Yeah, it does. My challenger question about that would be when you are looking through that and you've put your mind into a specific mode for a specific commit, haven't you narrowed the focus of your brain then? Because if you're saying there's only things in this commit for this feature, you might actually tell your brain to ignore something else that's snuck into that commit, or you might not notice it. I mean, I know what you're saying. You might notice it because it's not of the description. But I'd say that people tend to shut off their brain when they focus on something. I mean, it goes both ways. Some you might notice the outlier, others if you're focusing on one thing that's the only thing you notice. If the test was, or if the PR was change all the tests from at_test to test as camel case or something, you might miss that you change a couple other things inside of one of the functions because you're just scanning for, looking at those function names to make sure they make sense.

That's not how my brain works, but I could see that argument. For me, the fact that I've narrowed my focus actually makes me more hyper-aware of anything that's outside what I'm expecting. If I'm really expecting, just like you said, snake case to camel case or whatever, and then all of a sudden there's a hundred of those and then number 75 changed a variable name in the test. I'm like, "Whoa. Why did that happen? That doesn't make sense to me." But it's very specific. It's when an atomic commit is really supposed to be one thing. If it's like, oh, this commit is a refactored tests. And some of it is changing the name, some of it is changing attributes and some of it is tidying up test logic, that probably wouldn't even look at that commit in isolation at that point. Because it's not really atomic in my view then it's doing too many things.

Let me hammer at this one thing one more time before I tell you how I work.

Good.

I would say that I think you're putting some trust into the developer then that they're categorizing their changes and you're allowing them to put your focus in specific places. Whereas the point of code review is that I catch things that you aren't thinking about. If you're constructing a set of stuff and you're saying, "Focus on this," as a developer to your reviewer, and then they follow that path, isn't that just you looking at it again?

I don't-

I mean, no, it's not. But I mean, there is a... I don't like when someone comes and says to me as a reviewer, "Here's what I want you to review." It's like, "Well, then do it yourself."

No, I get what you're saying. I think I can see a parallel, but I think there is a distinction. Like what you just said, I would put something in a PR note. Like, "Hey, this thing here pay special attention to that." Or I might get ahead of it, "Here's why I did this." I wouldn't take an instruction like that and be like, "Oh, well, that's all I'm going to look at." I get what you're saying with the commit. I am sort of adopting that mindset of like, "This is the main thing I'm going to look at." But I would just tweak it a little bit and say, "This is the thing I'm expecting. I'm still looking at everything." But yeah, it's very subjective. Because even what makes an atomic commit is a subjective thing too so I don't think there's a hard and fast rule.

I think this is a great example. You said a really cool phrase just in passing, which is, "That's not how my brain works."

It's not.

And that's great because we need to, as developers, recognize that we're doing the same coding maybe but our brains may work differently. We're accomplishing the same things, the end result, even some of the path might be the same, but the way we travel that path. Maybe Joel likes to skip down the path and I like to walk. There's all kinds of different ways to get there. The reason I thought that was interesting is, you're like, "My brain doesn't work that way," and I'm just over here screaming, "Mine does." You asked me how I do code reviews. I look at the feedback from the PR note. So if you like, "Focus on this," or "I did this," or whatever. I'll do that. I'll do something strange. Is I might not even read the ticket that's for until after I've done all the review and I'll tell you because here's how my brain works, maybe it's different. But you know Tetris?

I do.

The game Tetris?

Absolutely.

You know, as you're building your Tetris board, there is this idea that you can put down different shapes and they kind of fit in. You know, I need the long four square thing so I'm building everything on the right waiting for that one to come to the left, and whatnot. So you have this idea in your head that as new blocks come in, you fit them into a picture and that picture continues to grow and grow. And then some comes in and maybe that lowers the picture, it changes the picture, a couple lines go away, et cetera, but you continue to build on top of it. That's how my brain works. Is when I look at code review, I don't look at specific commits or whatever, I just start from the logical top, which is almost sort of alphabetical. But sometimes I'll jump around, I'll start in the controllers and then I'll just look at all the controllers. And the way my brain works is as long as I'm not interrupted and I can specify like, "I have 10 minutes for this," I can keep all those things in my brain. I'll look at other controllers and then I'll give feedback on there. If there's something I don't know, I might put in a temporary comment. Say like, "Look back at this." So I haven't submitted that comment, but I'll put a temporary one there. And then I'll go down, I'll go down, I'll find the answer, and I'll delete that temporary comment if it no longer makes sense. Or I'll leave it there if it did make sense. I might even edit the comment and be like... Well, in the original comment it might say something like, "Will this work with X, Y, Z?" And later on I discovered that model, I may go back to the comment and put another comment on there saying, "I checked the model and it doesn't appear that this would work." So that's how my brain works, is I look at the entire thing and I start to build like a Tetris. I don't want to be focused, I want my brain to do the focusing for me. So when someone says, "Focus on this," that's when I get frustrated because I'm like, "I don't want you to lead me that way because my brain doesn't work that way."

Right.

I think some of the beauty of us having different brains, just developers in general, is like I said, we're all going towards the end result but we get there different ways so we get to discover different things along the way—different ways of coding, different bugs that might've come up, whatever. So when you try to focus someone too much on a code review, like me, they might miss the things that they would've caught otherwise.

There's a couple of things you said there that I liked. The first one that really resonated with me was actually leaving yourself notes in the PR process. I've never done that but now that you say it it seems obvious. Because it does batch them, it's not like-

Oh, I do that all the time.

That's great. It's like a little sticky note as you're going through the code. The other thing too I realized the downside, the thing I've bumped into trying to do atomic commit review is sometimes there is a bug in that commit and the developer finds it later. You're like, "Oh, you forgot to validate this," or, "You didn't account. This should be in a transaction," and a later commit they did that, right?

Yeah.

That is a problem. And I would say honestly, less than 10%, maybe 5% of the time I look at them commit by commit. But there are some cases where it's like big global style changes or refactor or things, I do still kind of like that but I think your way makes more sense, Aaron.

Well, I don't know if it makes more sense. It's just that's the way my brain works. Again, because when you mentioned a huge refactor and then looking at atomic commit, in my brain that is useful information that I might forget if I go commit by commit. If I'm looking at it all at once, I now can put myself a little bit in the, sounds so silly, but the emotions of the developer as well. Like, look at all this stuff they're changing so if I was changing all that stuff, what are areas where I might make a mistake? Realizing it's not necessarily in the area that I'm working on, but if my task is 80% refactoring something, 20% doing a task and I see a pull request of 80% refactoring, 20% code, that tells me right away that I'm going to look at a 20% code really much more in depth. And I only know that because of the ratio of one giant PR. But if it's individual, I might not notice... I might not remember or I might not notice.

True. I like when you said emotions because there've been times, because we'll do an interactive code review sometimes. Where you leave all your comments and then instead of going back and forth, we'll just look at all the comments together. And you'd be like, "Yeah, Joel, I could tell you were just getting frustrated here." Or, "It was getting late in the day." And it's like, "Oh, you're right, Aaron. Dang, how did you know that?' I have a question for you Aaron and you have to not react badly to it. Because I know you don't like cheese but this is a cheese-related question. But you don't have to enjoy cheese to answer this, this is a more philosophical question about food in general.

Okay.

I was at a cheese store and they had a section with aged cheeses. You know that's a thing, right?

Yeah.

It's like wine. Oh, an aged... or steak.

Actually, I know a lot more about cheese probably than the average person because people attack me with cheese.

I know. That's why I-

Because I don't like cheese. And they're just like, "Well, have you tried this cheese?" I've learned everything. "Well, have you tried this X, Y, Z cheese?" I'm like, "No. But also I don't want it."

That's why I'm being very cautious with this question because it's not that at all.

Okay.

This is an age cheddar, 18 years old. It's like... I want to say like $100 a pound and it was a little bricks, small pieces. But so I'm like, "So this cheese..." We're recording this in 2025, that was from 2007. Wow, that's older than my kid that was standing there with me. But then on the line underneath it, it said, "Sell by January 2026." And I'm like, "It goes bad?" It's fine for 18 years, but if it goes a few months more throw it out, it's bad. Then right next to it, there was a 20-year cheese. I'm like, so clearly cheese can go to 20 years. What is your take on that? For an aged product, why would there be a sell by date? That's my conundrum that I'm trying to solve.

Because you age stuff differently than you package it for selling.

Say more about that.

Well, I assume that when they age cheese, it's not in that original package. I assume it's in a giant block in some sort of warehouse with all the other sad cheeses spending its life in there. Just hanging out, going like, "I wish someone would eat me," and the other cheese says, "Quiet, just hang in there." So when they finally bring them out, it's a giant block of hard cheese of age 18 years that kind of has a little bit of dog hair on it. Rolls down the driveway and the guy pulls it out, starts scraping off the outside, chunks off little bits, puts it in the plastic, and says, "Yeah, that'll probably last a year. Eat that."

You paint a very vivid picture and I hope that's not what happens, but I think you actually explained it for me. Yeah, it's aged in a special room. It's part of a bigger, I don't know... Cheese comes in wheels or something? A bigger structure. But once you cut it out and now it's on the refrigerator in the store... okay, I think you sold me.

You want someone to just stop talking about code review and actually do it for you.

That's a service we offer. If you're interested in having us look at your code, head over to masteringlaravel.io and schedule a call with us or use the contact form.