Ask HN: What is a common PR review time at your company?

As a reviewer... it really, really depends on how trivial or gnarly the PR is.

If it's a simple change that is obviously correct, I'll try to unblock the author ASAP - often within minutes, even if it interrupts my flow.

If it's a giant PR with lots of risky changes in vital code, an awkward-to-unreadable diff, and/or maddeningly questionable design/coding decisions that require me to think a lot and compose some nuanced, reasoned comments to steer you in a better direction, then, well, you might find yourself nagging me for a review 2 days later. (And I'll probably ask you to split up PRs like this in future, e.g. to separate major refactoring from any logic changes.)

Here's what no one will tell you about PR reviews. Highly productive engineers often learn to work around them. Once you've been around for a while, you gain trust. This usually results in a group of people who will rubber stamp anything you send them.

I've been doing this for 20 years and I've never seen an organization where this didn't exist to a significant degree.

I review my PRs myself line by line. And because of this I've literally never had a PR reviewer catch a major bug (a bug that would have required declaring an incident and paging an on call engineer had it made it through to prod). Not a single time.

So the vast majority of the time, I don't care that someone is going to rubber-stamp my PR. Occasionally, I'll ask someone who knows more than me to give it a thorough going over just in case.

I also very rarely have someone suggest a change in PR that was worth bringing up. But most of the time if I'm doing anything non-trivial, I've already talked it over with someone else well before PR time. I think that is the real time when collaboration should be happening, not at the end of the process.

Essentially, the PR a process at most tech companies is nothing more than a gatekeeping/complicance/CYA mechanism, and realizing this can definitely help your career..

At the last place I worked it was common to wait one to two weeks for a PR review. I was the only developer who would do a review within the same day that someone requested it; as a result everyone came to me when they needed a review to actually get done.

When performance reviews came around I was criticized for not getting enough story points merged (because I couldn't get them reviewed) and the fact that I was single handedly responsible for reviewing almost all of the PRs was not deemed to be a valuable contribution to the team.

Code review should really only exist to guide contributors to the owner of the code they are changing. A new contributor shouldn't need to know the team structure and history of the organization to get changes in. That should be solved by the tooling. Do whatever you want, and the tools will force you to bump into the right people before you can merge.

It's very strange that most companies (maybe GitHub is to blame for this) have something like a free-for-all where anyone can approve anything, as long as they themselves aren't the author. As other comments have mentioned, this reliably creates reviewer cliques as engineers seek out the path of least resistance to get changes in. Those who haven't figured out the game often lose hours of their time to nitpickers and pedants without anything better to do.

This also results in a slow decay since most of the code ends up being written by people who are good at getting code merged, and not people who are good at building the software.

Has heavily depended on employee incentives.

At some employers, particularly the Scrum point counting ones, PRs were unrecognized work, so everyone avoided them unless hounded and then people traded PR approvals at the end to score more points as “done.”

At employers that didn’t care about points complete per sprint, it depended on the overall importance of the work as the team leads jumped in to have them done. But even then, as it wasn’t recognized as work in any way, during crunch it got abandoned.

It depends on the specific coworker I send it to. I have coworkers who will review within an hour without any additional prodding, and coworkers who will take up to a week unless I prod.

In general for small PRs I consider a consistent latency of over 24 hours to be long and mention it negatively in peer review.

Our standard is that when a reviewer is no longer in flow, review should be the next thing they do. The SLO is 1 business day, which is supposed to be the maximum, and the guidance suggests it should be far lower than that. In reality, it varies wildly by team.

I find most of my PRs being reviewed by my team, on the same timezone, are reviewed within a few hours, but I suspect a big part of that is that most of my PRs are very small (they're actually CLs not PRs, and closer to a single commit in some ways, although hard to draw specific parallels to Git).

It depends on the language/ecosystem a bit, but normally I'd try to keep changes under 100 lines for an efficient PR process. Up to 500 lines is manageable but much slower. Over that is just impossible to do good review on.

Something that my previous company had a lot of success with was review by commit. Making sure that the series of commits in a PR "tells a story", with each one being simple and obvious, and then instructing reviewers to review commit by commit. It can speed things up and increase the quality of review substantially, but does require a bit more work from the author.

If your PR isn't blocking someone nontechnical then I expect you to use developer tooling (i.e. VCS) to continue working without requiring that your PR be merged immediately, and I'll review the PR the next time I come to a natural break in my work. If it is blocking someone, then I'll review ASAP with consideration to the priority of whatever I'm working on.

I work with a guy who, until I said "calm down", would share his PR links in the team chat several times a day. He refused to learn rebase, or seemingly anything other than a standard Git workflow of checkout, commit, push, merge, repeat. He has since improved on both fronts.

I've always established the team rule that most PRs must be reviewed within a day. At the very least, devs should be reviewing any open PRs before they leave or when they first get in.

Obviously, there's going to be exceptions for large changes or big rewrites on the one hand, or for tiny label changes on the other hand.

PS. Reading the comments here, people seem to work in some pretty dysfunctional environments. Get the team together and establish some guidelines that everyone agrees on. Review time should be predictable, and the work should be reasonably shared.

On my team we expect that everyone looks at PRs at least once a day. If something is particularly pressing, they can post a message to our internal team channel for a faster response.

Inside my team, we generally review quite promptly (same day, or the next morning). Across teams — it's more variable and depends a lot on context (or lack of context). If I'm chasing down reviews, I'll usually tag people to start out, and then message them directly to ask for reviews 24 hours later, if they don't get back to me sooner. It's rare that someone doesn't review same-day if I ask them directly.

I often find the reviews to be helpful. People come up with edge cases I haven't thought of, or notice things to clean up. I try to proofread everything before asking for reviews, but after I've started at my own diff for a while, it's hard to read it with fresh eyes.

Sometimes we ask for reviews on more experimental PRs too, just to get early design feedback before wasting too much time on polishing the changes.

The Firefox team’s code review guidelines recommend:

> Aim for a response within 1 business day. This need not be a full review, but could be a comment setting expectations about when you’ll be able to review the patch, pointing to other folks who might be able to review it sooner or would be more appropriate to review the patch.

https://firefox-source-docs.mozilla.org/browser/FrontendCode...

To speed up reviews that don’t require a particular individual’s approval, some Firefox teams create (Phabricator) reviewer groups for different feature areas (such as graphics or Windows code) so anyone in that group can approve a PR.

The biggest issue with PR reviews in my experience is not how long they take, but the justifications people provide for their feedback. I don't think you should ever leave a comment if you don't have a convincing answer to "why" you left it.

The change request must either make the code objectively better (i.e. performance, memory, security, adherence to a consistent architectural pattern, simplification), or subjectively clearer (it's hard for you to understand what is happening, how it happens, or why it's done this way).

If the reason is solid, then the feedback is usually well received. Except in unfortunate environments where PRs are treated as annoying disruptions. I hope I never have to work in that kind of place.

We expect same day or next day response. Of course only if you're in. But if you're not I might reassign. Also if it blocks me I will ping you in less than a day or straight away.

My most common response is "needs no further review if these points are addressed". On rare occasions my only response is "need more time". Some changes take time to settle in my mind. No good comes from rushed comments.

There is an expectation not to haggle over dumb shit. When you decide to pick a battle, pull in a second reviewer for their opinion.

I'm sure you've heard the "trick" of splitting up large merges to make them easier. Yes this is good up to a point. Other than that, it really is team commitment to timely reviews. No tricks to it.

In my department (DevOps/Infrastructure/CloudOps or whatever you wanna call us folks who write terraform and helm) we have a couple of set standing weekly meetings that we all get together, and one of the frequent things that happens is PRs get brought for group review. So, the longest anyone has to wait is a couple of days.

We also have a dedicated thread for PR review requests in our private team chat channel, which helps bring visibility to our-team-specific PRs that can get lost in the churn of github notifications from all the repos we’re involved with.

on my teams, I've set expectations that people look at PRs between tasks. Don't stop flow for it, but don't jump straight to a new task or come back from a break without making sure nobody is blocked waiting on reviews. If the whole team follows that method, and keeps their PRs fairly small-ish, PRs typically fly through the system just fine, and it also spreads the PRs across the team.

We've gotten to the point at my company where PRs are almost a formality. PR -> merge -> Put on environment for QA. QA finds issues, fix issues with new PR, merge, and so on until we have our release.

My last company was much more methodical with actually reviewing code but honestly? I kind of like the trust in developers and we're able to get code out a lot quicker.

On all the teams I’ve worked on the written standard has been one business day, but most people get to them faster. I always make sure to tell an author if I don’t think I can review their code same day. Just a comment saying “hey this is a big PR and I’m busy, ill get to it tomorrow if nobody else beats me to it”

As a reviewer, I've been guilty of letting a PR sit for a month, mostly because there were higher more critical things that needed my attention.

Lately, I don't want code to be perfect, because my code never was in the first place. I want it to be just good enough for me or anyone else in the team to iterate on.

Stacked pull requests solve this.

It doesn’t matter how much you wait for review of a PR if you can just build on top of it.

Also, this workflow encourages smaller PRs that are faster and easier to review.

We implement this at my company through a tool called Graphite and review times usually don’t exceed 1-2 days.

It depends but if I break them down then same day. If large I need to beg.

To me the ideal system would have a reviewer assigned at the start that is expecting to review and you keep them updated on the design.

The median case is probably that the author shares it as ready for review, and so the review is ~immediate.

I hate that. Mine have a much slower time to review; sometimes I eventually give in and share them too.

Typically no more than a day. But let me caveat it by an anecdotal observation that the duration is roughly proportional to the cube of LoC(changes).

The real question is, are you pulling down the code and running the PR locally before approving it, or just looking at the code.

Work that requires coordination of independent schedules takes longer than the theoretical minimum.

It's a queuing problem. See: https://www.johndcook.com/blog/2008/10/21/what-happens-when-...

The slowness of processes that involve other people will frustrate you until you realize it is an ordinary part of the work.

My advice, is behave in a way that makes you the kind of person other people will want to work with. Turn your work around quickly. Be patient with other people's turn around time.

Good luck.

I totally agree, waiting a long time for PR reviews can be so frustrating! This used to be a pain at my company too, but we've been able to dramatically improve the review time. We target getting a review, on average, within one hour of posting a pull request. Most weeks we now achieve that (or get very close)!

I think it comes down to two main pieces: culture & tooling.

Culture: - Reviews are part of the job. A solid senior developer isn't just writing code all day -- there are many aspects to the job and reviews are an important one - Reviews shouldn’t be viewed negatively or as something we "have to do." They’re opportunities to learn, teach, collaborate, and improve as a team - Make small, well scoped PRs. Do not try to roll a bunch of changes into one PR. It makes it harder to review, riskier to merge and harder to rollback. - Reviews should be owned by the team. Often there doesn't have to be just one developer who can do a review. If a PR is waiting on a developer who has a morning full of meetings or is fixing a high priority bug, then another team member should pick up the review - Prioritize code ready for review over code still being written. Code in review represents significant business effort (customer feedback, planning, design, development), but its value isn’t realized until it’s merged. Prioritizing reviews can help deliver that value faster - It’s awesome as an author to get a quick review, so "treat others as you’d like to be treated." - (Likely not feasible everywhere, but helpful for us) Skip reviews for trivial changes like copy updates, test fixes, or non-critical dependency upgrades

Tooling: - Automate as much as possible. Linting, tests, and other automated checks can reduce manual review effort - Make sure there's a process for everyone getting notifications quickly. Turned out some devs would use github notifications and only checked those like once a day, while others got real-time slack alerts (guess which group tends to complete reviews faster) - Have a way to set & track your review time target as a team, try to make an automated way to keep the team accountable & aware of where they're at

Obviously some of these things aren't easy to change and take time. I'm fortunate enough to be a TL so I was able to influence my team and the engineering department more directly, but I think a respected developer can bring forward this kind of change too. To also address some of these points for our department, I built a Slack <> GitHub app to help our team move reviews faster. Feel free to try it if you're interested: https://pullpro.dev I’ve been working on making it accessible outside our org and would love to hear any feedback if you give it a go!

My team tries to stay on top of code reviews as they come in, but this has been the practice as a very small team that has worked together over the course of three years. Generally we trust that each developer is running automated tests, manually testing and refining in a local deployment, and has the right context and knowledge for what they're changing. I can have code reviews open for hours or days, but it's a red flag if it's more than a few days.

However, we're onboarding a lot of newer developers and actively trying to slow this pace now. CRs need a lot more scrutiny in this case.

The second Friday of every month.