I review a lot of code with the mindset of yes and…
Basically when I start the PR is approved in my head until I find something blocking. I.e. major problem that causes dataloss, big performance issue or breaks other code. Anything else is a minor comment at most. The PR is approved by default. This gives the dev responsibility and ownership. Plus it increases release cadence
Doing a follow up PR to improve or fix something is just as fast as blocking the MR, but blocking the MR can be bad for morale.
This strategy might work better in young startups where having the feature _exist_ is better than not shipping.
in my experience this builds up responsibility and ownership and removes the whole policing of other peoples work vibe around code review.
Also decisions and discussion around formatting, when to test, design, features, functionality, architecture should not happen during code review, they should have happened way before coding, or collaboratively while working. Code review is the worst time for that stuff, imho it should be to sanity check implementation.
Basically when I start the PR is approved in my head until I find something blocking. I.e. major problem that causes dataloss, big performance issue or breaks other code. Anything else is a minor comment at most. The PR is approved by default. This gives the dev responsibility and ownership. Plus it increases release cadence
Doing a follow up PR to improve or fix something is just as fast as blocking the MR, but blocking the MR can be bad for morale.
This strategy might work better in young startups where having the feature _exist_ is better than not shipping. in my experience this builds up responsibility and ownership and removes the whole policing of other peoples work vibe around code review.
Also decisions and discussion around formatting, when to test, design, features, functionality, architecture should not happen during code review, they should have happened way before coding, or collaboratively while working. Code review is the worst time for that stuff, imho it should be to sanity check implementation.