alt Hacker NewsI'm not sure how the best teams do PR review..
They don't do them. They discuss the changes as the team are doing them (software design and architecture), they automate all the things that would be nitpicks (linting, formatting, etc), and they adhere to a strict 'Do not break the build' rule so merging in something can't be disastrous (with a lot of automated checks and tests to prove that's true). They also make sure there's a robust rollback process just in case.
Once you do all that the PR process is pointless. It never catches anything useful. The team can trust one another to merge without that gate.
Replies
> They discuss the changes as the team are doing them (software design and architecture)
I find the format of PRs to be better suited to discussion than slack/teams messages. Key developers need to actively sign off, code suggestions are much easier, and the CICD makes sure we're not wasting time discussing code that's not compliant with obvious standards.
Obviously some level of agreement is needed before creating a ticket at all, but implementation details around say creating a terraform module to share functionality between teams do need to be discussed somewhere - and it can be difficult for any individual dev to have all the low level context.
Thank you for this response. I do think this hits home as the most elegant and reasonable approach. For my teams currently, I can see us already moving toward this implicitly, but perhaps needing to take a more explicit approach. I think your framing crystalises it well.
> Once you do all that the PR process is pointless
manual PR reviews can catch things that llms currently miss. Examples are duplicated code, lack of unit tests or introducing security issues. None of these really break the build. So just requiring "do not break the build" is a very low barrier.
Tests also are useless unless you have a smart system that runs the new test WITHOUT the changes and see it break. Most teams I see today have the LLM write a test along with the change, without a guarantee that the tests actually guard the feature.