Pull Request Reviews

Do you find it takes more time to review a PR than it took for someone to figure out the solution and implement it?

Ask HN,It feels increasingly backwards that PR reviews are where we catch basic bugs, style issues, and convention violations.With AI writing a large chunk of code now, the volume of small, avoidable mistakes has gone up. Yet we still wait until PRs to point out things that could have been flagged instantly, pre-commit, on the author’s machine.PR review should be about design, correctness, and tradeoffs — not “missing await”, inconsistent patterns, or obvious slop.Is there a good rea

The problem is other people/teams making PRs to your code that you then have to maintain or fix later. It’s in your interest not to half-ass the review, creating an asymmetric amount of work for you vs them.

As someone else mentioned, the process is async. But I achieve a similar effect by requiring my team to review their own PRs before they expect a senior developer to review them and approve for merging.That solves some of the problem with people thinking it's okay to fire off a huge AI slop PR and make it the reviewer's responsibility to see how much the LLM hallucinated. No, you have to look at yourself first, because it's YOUR code no matter what tool you used to help write

I've said this before, but I work on a team of 5 or 6 people. If I (pre covid) sent them a PR and walked over to their desk, told them it was super urgent and a tiny change just needed a rubber stamp, one of them would do it (and I would likely do the same for them). Failing that I can name a handful of developers that wouldn't be familiar with the system but will review my change because I did the same for them a few months ago (and they'll comment on stylistic/clarity issue

Bad idea. I think this bit near the top shows that the premise is incorrect:the adoption of Pull Requests as the primary way of contributing code has created problems. We’ve lost some of the “Ready to Ship” mentality we had when we did Continuous Integration. Features-in-progress stay out of the way by delaying integration, and so we fall into the pitfalls of low-frequency integration that Continuous Integration sought to address.PRs and code reviews don't need to cause major s

PRs on my team are done at the request of the author, who maintains full responsibility for the code. When review is complete, the author merges when they are ready (they have full access rights). The author has the right to ignore whatever feedback they want, since they will share the burden of whatever bugs are introduced. However, merging bugs knowingly and/or ignoring feedback is frowned upon.Reviewers are expected to provide whatever type of review the author requests, including man

Yes!I’d also add that it’s helpful to have a review guideline.When I get to the PR phase, I’ve already done the design work, considered the trade-offs, and am asking folks to check that the code is mergeable per our guidelines.A pet peeve of mine are reviewers with a bone to pick who will leaving blocking comments to redo the work in the way they would have approached it.A guideline helps here because you can, along with your team, manage expectations for the process.Some folks li

Does anyone actually review PRs commit by commit?

Code reviews help gate this behavior, a bit. If a coworker does this and drops a 40k line PR, they get code review duty for everybody else for a little while _before_ their code can get merged. Letting them go unfettered definitely is a mistake.