3 ms·
While I think code reviews are great, I think the ideal state is doing them asynchronously post-merge. Enforced pre-merge reviews are a way to hold the change h
by epage 4y ago
While I think code reviews are great, I think the ideal state is doing them asynchronously post-merge. Enforced pre-merge reviews are a way to hold the change hostage to force a certain quality bar. If there is the experience and trust on the team, you don't need to hold each other hostage. You know the author won't make too much of a mess, you know someone will come around to doing the review soon, getting the benefits of code review, and you know the author will be responsive to feedback and not just dismiss them because they moved on.
Even when you do have pre-merge reviews, some times the culture and tooling can make things slower than they need to be. I saw this at one company:
- Review system sent so many notifications, people ignored them
- It was unclear if a change was relevant for you to review
- When there is communal ownershipg no one owns it. The code review system needs to assign the reviewer from among the team
- it was unclear when you owned reviewing something
- slow CI and not having auto-merge makes people lose track of things. As a last resort consider a two stage CI, fast build of a subset for PR updates, full build on merge.
- people should treat CI as another reviewer and not block on it