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.
Why would the answer ever be anything other than 5?
Let’s just go full boar hypothetical: Someone is trying to merge malicious code. Anything other than 5 means the malicious code gets merged.
I can only think that what they have in mind is a small project with only few people involved. Otherwise anything other than 5 is ridiculous.
In a small shop where people really know and trust each other and all have high quality standards and would never break main - is code review necessary and for what purposes if so?
Even well intentioned people can make mistakes
Yes, absolutely.
“Never break main” is the same concept as “never get in a car accident.” Good in theory, but it’s no replacement for insurance.
Everyone makes mistakes. PRs help catch those mistakes. Yes, bugs will still sneak in, no one is perfect, but a proper PR process is absolutely vital no matter the team size.
Yes, it’s necessary. Even if everyone writes perfect, bug-free code, people learn from code reviews.