Item — HN Reader

> You cannot do it in one pass

> If you want code reviews to be good you need a policy that the reviewers need to approve it 3 days in a row without changes

I may be misunderstanding, but it sounds like you’re dealing with very complex PRs/commits.

I think the first problem to solve is get your commits to a reasonable size, so that you don’t need 3 days worth of approvals and multiple passes for review. (That sounds insane to me!)

My typical commits are maybe 100-200 lines (many of them much smaller), so needing 3 days of approvals would be way overkill. And a single pass is plenty.

1 comments

Wait, your commits are hundreds of lines, or your MRs/PRs? I like to keep commits as atomic as I can, not always succeeding, but often cutting commits to leave the repo in working state after a commit. Very rarely commits become 100 lines of changes. Perhaps those are variable renamings of things that are used in many many places? Or perhaps some wip commit, just making sure work is commited at the end of the working hours?

A single pass often cannot work, if there is a single improvement to be made, since that implementation of the improvement needs to be reviewed again, unless it is already literally a suggestion of code by the reviewer.

For me, changes are as small as possible, however it depends what I'm working on. Currently I'm doing major refactors to get a 1.5 MLOC monolith codebase onto a new runtime. This involves significant refactoring. You don't want to change many things at once, so one refactoring for example might be to make a commit which removes a pattern that will no longer exist in the new runtime. That can easily be 2000 files changed for a total of 10,000 lines. The commit is atomic and does one thing and would not make sense to split up.

With that said, even though it's the same atomic change in each file, the reviewer would still need to check them all.