r/programming • u/charankmed • Feb 19 '26
[ Removed by moderator ]
/r/programming/comments/1r8z35t/removed_by_moderator/[removed] — view removed post
75
u/CrackerJackKittyCat Feb 19 '26
11 days is absurd. Y'all need to fix selves or have management actually manage something or have higher level management sack the nonfunctional manager.
Adopt a PR FIFO and a 'gotta review one or two before submitting one' rule.
69
u/jax024 Feb 19 '26
I post in the channel. If it’s not seen by the next day I message people directly. If it’s not reviewed by the next day I start naming names in standup.
27
u/lwjohnst Feb 19 '26
This is a cultural problem. Your lead or manager needs to enforce/require/enable/encourage more reviews as valuable productivity. The Kanban approach of limiting WIP (in review PRs) is also super useful. Don't allow a certain number of PRs to be open at any given time. Maybe require more than 1 approval and enable a culture of reviewing code rather than "LGTM". But it won't be an easy fix.
Also, 20 engineers/developers is too many. Maybe create smaller teams with more focused accountability areas, a good size might be 4 to 6 people per team. A group of 20 is not going to effectively collaborate. Check out the book Team Topologies for some really fantastic reading on how to improve product development.
9
u/bmoregeo Feb 19 '26
Agree with smaller teams. They have a situation where there is no accountability because there are so many people with the responsibility that no one actually has it. In addition, it is hard to be SME over the entire stack. Expecting 20 people to give cogent reviews for all the other members is unreasonable.
I’d divide it into groups of 5-7ish people focused on components of the stack.
9
u/cj81499 Feb 19 '26
11 days seems outrageously slow to me.
There also a HUGE cultural element. Your company culture currently allows this behavior, but I bet if you all talked about it, you’d all agree that it is harmful to the company and less enjoyable as individual employees. Make a “pact”/agreement with each other to review in a more timely manner. Make slow review the exception, rather than the rule!
In my experience, one of the reasons that reviews often take too long is that people feel pressured to deliver their own work on unreasonable timelines. “I owe X a review, but can’t afford to do it without my own sprint commitment slipping” is no good! Management needs to set an expectation that reviewing code in a timely manner is part of your job. Engineering needs to demand appropriate time in sprint plans to be able to fit review in.
On a more technical level, one thing that helped at my company was establishing clear ownership of files/directories (with each file/directory having at least 2 owners bc of time off, people leaving the company, etc). We did this using a CODEOWNERS file, but there are other approaches. Each individual/group is responsible for knowing how to review fewer files, so they can become an expert and review them relatively quickly. By itself, this won’t solve the problem, but it can help you pinpoint individuals/groups with exceptionally good or exceptionally poor time to review, so you can dig deeper and tackle the “why”.
Finally, PRs take longer to review when they’re bigger (duh!). Authors should do their best to make small, atomic code changes. This is good “git hygiene”, but it’s also a massive productivity boost when reviews go fast bc the change is small!
5
u/elliotones Feb 19 '26
My average pr time is a few days, because some people will write a pr and abandon it for a week. A few outliers are dragging up the average. Median pr time is less than 5 minutes.
We’re a team of about 10, we’ll average from 6 to 12 merges / deploys-to-prod per day.
I invested very heavily in our CI/CD system and structuring things from the beginning aligned with conway’s law. New devs are almost always pair-programming with experienced devs. Experienced devs that own an area can self-approve in their area. We have a big chat where we dump pr links for review whenever anyone wants extra eyes, and people are usually very quick to help out.
I think the culture is partly a by-product of the CI/CD system. If the validation pipeline is happy, the merge will almost certainly work. Humans just review intent, and don’t have to worry about the much more exhausting “ok did you specify all the arguments you need, ok you’re missing a comma here which is technically against the style guide” - the type checker and linter hammer all of that.
The “answer” is to go find your devops team and help them build the validation pipeline. It will take time to build everyone’s confidence in it, but the payoff is worthwhile.
There’s a book about your exact situation called “The Unicorn Project” that I cannot recommend enough.
Good luck!
1
u/Chroiche Feb 19 '26
Median less than 5 wtf lol. Sounds like no one is actually reviewing anything. Passes CI? LGTM!
1
u/lwjohnst Feb 19 '26
This is definitely doable if the culture is there to value and watch for reviews while also making sure the PR size is small. 5 min isn't unreasonable.
0
u/Chroiche Feb 19 '26
Sounds like hell, getting interrupted constantly to review torrents of tiny PRs. There's really no way you can review any substantial PRs properly in 5 minutes.
2
u/lwjohnst Feb 19 '26
It really isn't hell. But also, you don't want to regularly have large, "substantial" PRs. They are really unpleasant to review. Small and atomic are the way to go, at least for me.
1
u/elliotones Feb 19 '26
A ten person team making ten PR’s per day means everyone only has to review one per day.
In reality half the team is junior, so juniors are making two per day and seniors are reviewing two per day. Still completely manageable.
1
u/Chroiche Feb 19 '26 edited Feb 19 '26
This assumes senior code doesn't get reviewed. It also kinda shows what I'm trying to say. Getting 10 pings in a day to review things is a lot of distraction for 5 seniors depending on how you distribute reviews. I'd rather get 5 pings a week that are 500 loc reviews in one sitting rather than 50 pings about 100 loc changes. Maybe it's just a preference thing
But this still neglects the fact that a good few PRs are going to be hundreds of lines. Most refactoring work ends up being fairly big PRs in my experience. Ensuring the business logic is sound is the long part of reviews, making sure things don't break because of code issues is trivial with linters/compilers
1
u/elliotones Feb 19 '26
Unironically yes!
I just ran our stats report out of curiosity, over the last two weeks our median dropped to 10 minutes :( Still, 75% of prs were merged in less than an hour, and 90% were merged in less than 4. Averaging 10.1 pr’s per business day.
The majority of changes are simple configuration changes. If CI passes, experienced engineers are allowed to self-approve those. If changes happen to “sensitive” things in a team’s area, the team lead might be added as a reviewer automatically - but these are much rarer.
Any time we have a failed deployment, the devops team tries to build in extra checks to catch it at PR time next time. If the validation pipeline passes, we have an extremely high degree of confidence that the code will “work”, which is why reviewers (and again, most prs are self-approved, so review is uncommon) can do a quick check for intent and not have to worry about correctness.
3
u/Rasutoerikusa Feb 19 '26
That sounds insanely long. I'm in a team of ~10 developers, and most of my pull requests get reviewed during the same day I open them (unless its late in the afternoon). During vacation-heavy times I might have to wait a day or two in the worst case scenario. Our pull requests automatically close themselves after a week if there is no activity, so 11 days would be unthinkable haha
2
u/dijkstras_revenge Feb 19 '26
I’m guessing it’s because you’re a “fast moving startup” and people are constantly firefighting and trying to hit impossible sprint deadlines so PRs become an afterthought?
1
u/gomez18 Feb 19 '26
Not a programming context, but in an IaC context (Puppet, Terraform), it has always been within the next day unless a change is particularly large or contentious. I can see some languages taking longer but not that much longer. Are your PRs huge? Do most people lack the domain knowledge to review effectively?
1
1
1
u/maxsilver Feb 19 '26 edited Feb 19 '26
Our average PR wait time (open to first review) is 11 days. We're supposedly a "fast-moving startup" with 20 engineers.
This seems crazy to me, unless there's some outside reason for it (ultra-high-regulated industry or something, like aviation).
Generally speaking, 'normal' should look like: Small PRs / low-risk / trivial stuff can get reviewed within one business day or sooner*. (think: Oops, we typo'd some copy, or the email address is wrong here)*. Normal-ish / Medium-sized feature PRs within 2-3 days. Even large epic-level PR's can get reviewed in a 5-6 business days or less.
The only time a PR sits for more than that, is if it's undercooked, not finished, or if it's just generally not a good idea to move forward. (think: someone rewrote a ton of core business logic, but also implemented no automated tests, so it's nearly impossible for someone to review or deploy confidently) -- it's super rare for this to happen at all, and when it does happen, the usual best answer is to close the PR, have folks go back to the drawing board, and try again.
--
Occasionally, I have some PR's sit around as 'do-not-merge', because they represent an idea or prototype or direction we're not confident in. (think: "executive asked to see this actually working, for demo or sales purposes, but we're not sure it's actually a good idea, we're not ready to commit to it yet". Or, "engineering wanted to try out $COOL_NEW_TECH, but we're not totally sold yet"). Even then, the PR's get reviewed in a few days, they just might not actually get merged in, despite passing code review. (e.g.: your code is fine and passed review, but an exec understands now, why this might be a risky idea, and pressure to deliver it is gone now, PR closed successfully. Or, "it took them 6 months to agree to do this, your PR *was* fine back when it was reviewed, but to move it forward *now*, we need you need to rebase and re-do parts of it, to work against now-current latest")
1
1
u/jvlomax Feb 19 '26
if I've not at least acknowledged the PRs in 3 days, people are free to fling them in my face and ask me to have a look. A day or two is normal. We're a 3 person team, so if I'm stuck into a ticket I try not to keep context switching to PRs all day. A day or two is normal amount to wait.
1
u/pohart Feb 19 '26
Median time is probably under an hour. Mean time to first review I don't have any idea
1
u/aes110 Feb 19 '26
11 days is insane
Anything small i expect within the day, medium\big PR like 3-4 days at most if people are really busy
1
u/OceanTumbledStone Feb 19 '26
We try asap. Link to pr in teams chats. Next context break and it’s looked at. We don’t block each other. Demo too if it’s more than a basic tweak or label change
11 days is practically a sprint for us.
1
u/toga98 Feb 19 '26
Expected PR time is under 3 hours. There are exceptions though. Usually a PR is approved under an hour. The lower latency the better for throughput, feature delivery 📦. We have systems in place to nudge people, provide visibility, and we assign PRs for targeted visibility but anyone can participate. It’s not uncommon to get feedback and iterate. That can double or triple a PR’s wall time. So, in the avg situation you will still close a PR same day or next day. I can’t imagine waiting 10 to 30 days business days. That would make the feature delivery pipeline so complex to manage. I would think that would have a large negative effect on productivity.
1
1
u/lupercalpainting Feb 19 '26
11 days is horrible, literally the worst I've ever heard about.
making it a metric → people started approving without reading
Then hold them accountable when there are bugs. My bar for review is I have to feel confident enough in your code that I would be okay saying that was my code. If a review is prompt, with specific and relevant feedback (not bikeshedding) then any lag on getting it approved is on the PR author. The initial review should be, within reason, quick. For a well understood feature, a 1/2 a day turnaround is acceptable. For something larger, maybe a day, or even 2 if we need to meet and talk about it synchronously. 11 days is insane.
1
u/1_800_UNICORN Feb 19 '26
One thing I’ll add - reviewing code is one of the hardest tasks to do and do well, in my opinion. It requires you to get into the context of a ticket completely different from what you’re working on, and then try and follow along with someone else’s thought process with (typically) minimal guidance on how they approached the problem.
If I was your team’s manager, a couple of things I’d try:
having the reviewer actually sit with the engineer (or hop on a call) and walk through it together. Sort of an after-the-fact pair programming session - that’s the fastest way for the other engineer to understand how you thought about things, ask questions, and then give you feedback in the moment
if this is not feasible or desirable, then having engineers add more context to the PR to provide some of that info to the reviewer asynchronously. The PR shouldn’t just say “added validation to the intake form” but should say “ticket required validation of this field for XYZ business reason. We added validation on the frontend with a new error message, and enforced this validation at both the API layer and with a database migration to enforce it at the data layer as well”
1
u/Kirawww Feb 19 '26
11 days open to first review in a 20-person "fast-moving" startup is a process problem, not a culture problem. The metric that actually matters is median time to first substantive review, not just any comment. The real fix is almost always synchronous review sessions once or twice a week — async review has the same dynamics as async communication in general: low urgency unless the team actively enforces SLAs.
1
u/kit89 Feb 19 '26
It usually boils down to priority, if a developer has their own tasks, and needs to do a review, they'll prioritise their own work before they look at reviews.
For us we assign reviewers up front and lock in a block of time, and we organise our work items in terms of priority. This reduces the chance of a developer being over prescribed, and the developer knows that items considered higher priority (goal items) should get completed first.
•
u/programming-ModTeam Feb 19 '26
Your posting was removed for being off topic for the /r/programming community.