| Some of the pros as listed in the article taken point by point: > It also encourages the developer to keep pull requests small, ensuring a feature is developed in many small pull requests (or at the very least comprising of many atomic commits). I am of the opinion that pre-commit reviews also encourage small pull requests, if only to make it easier on reviewers -> easy review hopefully leads to fast approvals. > It also allows the reviewer to batch reviews, so they are reviewing a number of changes in one fell swoop, as opposed to being constantly interrupted for code reviews. This I can see being a potential benefit depending on the workflow. You could technically do the same pre-commit by working on a longer branch and putting that up for review as well. > In my own experience, this reduces nitpicking on the reviewer’s part, and in the event the reviewer does nitpick, it allows the developer to address the minor concerns at their leisure. Nitpicks are what they are, and shouldn't block a PR in the first place, so not sure how much benefit this would be. > If a design document has been clearly fleshed out and agreed upon before the code is implemented, the implementation will not comprise of any major surprises to the reviewer. That depends - the design may have been agreed upon by someone, but not necessarily by the reviewer. There may still be major surprises, which will be harder to fix post merge than pre merge. > This means that the reviewer can have a fair degree of confidence that the change is functionally correct and has undergone extensive testing before they review it. This seems brittle when there are lots of changes going on at once. It may be hard to narrow down if something works or not. > Building trust and learning how to work well as a team takes time. I think this line makes it clear to me where my difference in opinion is. I don't even necessarily trust my own code! |
I will often make a hedged critical comment, but approve the PR. I am capable of identifying my own "nitpicks" that are not functionally relevant. This allows me to provide my opinion, and track examples of where my opinion on a particular subject could have been applied, while not mucking up the process with a minority view.