Hey, I want your opinion on code reviews, what is the best way to use them in a professional environment? Pick one of the following and give me your thoughts (from the most forgiving to the most strict):
- no code reviews, they are useless
- optional code reviews
- mandatory reviews on code that is already merged, optional fixes
- mandatory reviews on code before merging (like a pull request), with a time-frame for optional fixes (i.e. whether to fix what has been pointed out is up to the author), merge will occur anyway.
- mandatory reviews on code before merging (PR) with mandatory fixes.
Of course in open source development with public contributions, you’ll often see (5), but I’m not convinced it could work in professional dev.
Edit: I’m talking about a team of 5 mid to senior devs (no junior or interns) working on a 2-3 year project without many security concerns, but feel free to give me your general opinion.
The way I see it, for any code review there are going to be different levels of recommendation regarding the comments. When I review, I try to make it clear what’s optional (/ nitpick) and what I’d really like to see fixed before I can approve it.
So even making some assumptions, I can’t choose between 4 and 5 because optional and “less optional” changes are often in a same PR.
The only one I haven’t done much of is #3. That one looks better if one has questions about code that was already reviewed, merged, and it’s likely in production.