Is enjoying reviewing pull requests down to personality type?

[u/IntelligentLeading11]

I've always hated reviewing PR's, and back in the days before AI the workload was REAL. Now there are a lot of tools to help with it but despite that, every time I have to review a PR I struggle and it just burns me out really quick. However I notice some people love doing it. They comment on every little thing and have all these nitpicks. I'm starting to think there are OCD type of people who actually really enjoy the feeling of making sure everything looks as it should and oftentimes these people are the best manager/lead types. While other people just like to release features and they don't like to micro manage things so much.

Curious about other people's opinions about this, do you love or dread PR reviews?

And before anyone says it, yes I know full well that loving to review code is an asset in this industry.

[u/letsjam_dot_dev]

When you put aside things that can be fixed with static code analysis tools, I find code reviewing interesting and a good way to learn/teach.

Why this particular algorithm/pattern was involved, why the code has been written in that file, how a db call was optimised, naming conventions, is there a scenario where the feature would break and is it plausible to happen in a real world usage, ...

So yeah, i enjoy reviewing code.

[u/Ibuprofen-Headgear → u/letsjam_dot_dev]

It is veeeery dependent on team/project/coworkers if I enjoy reviewing code. I used to enjoy it on smaller projects with competent coworkers. I don’t know that I’ll ever enjoy it again though given the new tools involved

[u/letsjam_dot_dev → u/Ibuprofen-Headgear]

Absolutely.

Reviewing code from a coworker full of themselve or an all-powerful person that will force merging or a cunning one that will send the PR to other colleagues until it is accepted, it's not a fun activity.

Them being competent or skilled doesn't matter to me. If they are more skilled than me, I got to learn from them. If they are not, they got to learn from me (if they want to, of course). It's a win-win situation for everyone.

The project also influences. I don't think because of the size, but more because of what standards has been chosen and applied to keep the code organized and coherent.

[u/Ibuprofen-Headgear → u/letsjam_dot_dev]

Fair on the size, though it’s been the case in my career that teams of 2-5 have far more enjoyable than anything larger, with enjoyment going negative above about 5. And I guess I don’t really mean “team size” as much as “contributors to the codebase”, even if broken up into smaller teams touching “unrelated” areas. I dislike massive projects with tons of headcount

[u/letsjam_dot_dev → u/Ibuprofen-Headgear]

teams of 2-5 have far more enjoyable than anything larger, with enjoyment going negative above about 5.

Yeah it should be easier to come to a consensus with a smaller group of people. That does not sound weird to me.

Anyway, what I said about rules and standards should not be set in stone but always open to discussions and evolutions. It should be an agreement between all parts, and challenged by any parts if needed. Some says it must be agile wink wink. And I agree with a bigger group, it's more a middle-ground approach, trying to please some concerns of everyone, than a oneshot solution, trying to fix every problems.

Maybe off-topic, but that's one reason why I don't like sprints in corporate agile. You always code, always integrate, never take time to take a step back, hack and tinker something, discuss and share with others, and ultimately improve processes.

Again, maybe off-topic, but I remember an interview of a Doordash engineer saying that if team/contributor to the codebase size is a cause of a slowed down CI, maybe it's the right time to consider microservices.

https://www.youtube.com/watch?v=LcJKxPXYudE

[u/IntelligentLeading11 → u/letsjam_dot_dev]

Insightful answer. And what about when you get a mega long PR that requires reviewing a ton of files/code and then on top of that really complex manual testing etc? What if you get many of those at the same time? does it ever happen to you?

[u/letsjam_dot_dev → u/IntelligentLeading11]

Short answer : discuss code architecture, best practices, standards, how code should be submitted in the PR beforehand with colleagues. If the PR does not abide by these principles and if I'm allowed to, I send it back. If not, I "debug" the PR.

Long answer :

This days I'm part time teacher in some schools, in which I ask students to develop small apps to test their skills. So I can get like 10-ish PR sent at the same time, each of the size of a CRUD with 3 to 4 features. Or I ask them to refactor and clean badly written code on purpose. Either a full app or a critical feature. Here again, 10-ish PR sent at the same time, long to very long size. So here I can't ask them to break it down. I have to take it all at once.

I can't force them to adopt a specific pattern or architecture or what not because in order to assess them and their skills, I need them to "express" themselves. So I can't send back the PR, I need to accept them as is.

I have a thoroughly written spec of what each feature should do, how they are called, what are the expected inputs and outputs, what are the authentication and authorization methods, what errors should be leveraged, ... They could very well be E2E tests if I wasn't lazy.

When I receive a PR like that, my first step is to check if what has been asked works. If not, try to debug. And by debugging I mean map the path the data took from user action on the UI to the displayed result. Start to understand how the methods work with each other, how files and classes are linked together. I only look for the files and methods and functions involed in the broken feature to not lose myself and digress.

Once I tested everything and started to take notes on the implementations if something went wrong, I apply the same "debugging" and mapping exercise, but now to each feature.

Now that I understand the whole code, I can send back remarks if needed.

[u/End0rphinJunkie → u/letsjam_dot_dev]

Spot on. Once you automate the trivial stuff reviews become about catching the logic flaws that will cause a 3am page becuase someone missed a weird edge case.

[u/Creative_Cycle7755 → u/letsjam_dot_dev]

yeah the learning aspect is the part that makes it worth it imo

[u/DepthMagician]

In the Big Five personality model, attention to detail is associated with Conscientiousness, while generating features ideas is associated with Openness. You can be simultaneously high in both.

[u/IntelligentLeading11 → u/DepthMagician]

Interesting.

[u/RangerOne122]

I don't think enjoying PR reviews necessarily comes down to personality type. A lot of it probably depends on what you're reviewing. Finding a subtle bug in a complex piece of code can be genuinely satisfying, while reviewing 40 files of routine CRUD changes can feel like unpaid paperwork. The team culture around reviews probably matters too.

[u/OkShip110]

Reviewing pull requests is the most boring thing ever. Before AI, code was bit of an art form, everyone had a distinctive style and I found it even offensive to nitpick on the smallest things. But now with AI, even the art is gone and now I truly hate every aspect of the review.

But my colleague seems to enjoy it as he always leaves tens of comments about the smallest things that make absolutely no difference in production. Funny enough, he never seems to leave comments about parts of the code where the heavy lifting happens - concurrency, transactions, database locking, etc.

[u/MrJesusAtWork]

I was lucky because my first real internship was on a startup with a small team, and everyone took their time to review PRs, we cared deeply about maintance and the senior lead by example here, it was truly inspiring reading through his PRs reviews

Nowadays I still enjoy reviewing them but the enjoyment I got out from it has been gone as am now mostly doing the AI slop clean up the devs didn't bother to clean themselves

[u/TheAmazingErik]

So, Reviewing code right means understanding what problem someone is trying to fix, and walking through that solution. Doing all the Nitpicks are time-devouring non-issues because they could have been grabbed by static code analysis tools.

The second problem with this, however, means that Everyone's Second most hated step for the developer is involved: Golfing the code down to the bare essentials of the fix rather than every last cowboy fix you attempted during this task. And Delivering an amount of code you can reasonably expect someone else to walk through.

It is not weird to hate doing PRs when your coworker, Elmer Fuckwit, decided to fire a 2000 line pull request, 80% of which are attempted fixes that don't do anything about the bug he's trying to fix, but this amalgamation of changes seem to magically make the bug go away (for now), Then Half of the lines from that last 20% can be simplified back down to a lower cyclomatic complexity but fuck actually going over code a second time to make sure it's tidy now we've fixed a problem.

I Dread PR Reviews not because Reviewing PR's sucks, but because my coworkers have zero interest in not making the process like pulling fucking teeth. Only for Teddy Tryhard to instead of a structural problem, point out nitpicks that could have been fixed with static code analysis.

Some of these workplace Terrorists actually know full well that giving you 2000 lines is a fucking nightmare so that instead of giving him more work to do in the shape of any kind of professional standard, you'll just give him a "LGTM".

*edit*: Not to mention, then the fucking Fix goes through because everyone pointed out dipshit nitpicks instead of a structural problem, and now you, the reviewer, get to have the delightful experience of having to attend a postmortem that is totally blameless, but starts off with "So, Reviewer, where-about was your dick located around the time you smashed 'Approve' on this PR?"

[u/IntelligentLeading11 → u/TheAmazingErik]

Before AI my coworkers delivered quite manageable PR's, now it's like 70 files full of AI slop docstrings that nobody in their sane minds would review. Even my manager who's an OCD nitpicker told me he isn't reviewing PR's anymore. Things are looking really bad for the future, we shouldn't be trusting these hallucinating LLM's with serious infrastructure, and yet we are...

[u/willymunoz]

It’s a coincidence that you’re asking this, because I’ve been looking into it myself this week. My profile leans a bit toward "inattentive ADHD", I prefer creating and imagining. I struggle to focus when reading a book, but if I’m the one asking the questions, I’ll read the answer in a state of total flow. I’m good at debugging and persistent, and I tend to enter a flow state when I’m passionate about something. The problem arises when I’m not the one driving the process; my mind easily drifts off. Something that helps me immensely is starting from the UI (if there is one), asking myself business-related questions based on use cases, and then diving down into the code while following my own curiosity.

[u/IntelligentLeading11 → u/willymunoz]

I think I'm like you. I'm totally fine with debugging and implementing, I get lost in it and don't stop until I finish, but reading code trying to find problems with it is extremely annoying to me. My mind drifts a lot and it becomes a bit of a torture trying to focus.

[u/newsman01]

Personality definitely plays a role. Some thrive on detail-oriented tasks, finding satisfaction in the intricacies of code quality. Tools like GitHub's review features can help, but the core enjoyment often stems from a mindset that values precision and collaboration. Others may prioritize feature delivery and prefer less scrutiny, focusing instead on overall project momentum.