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.
All code going to the main branch must have a corresponding pull request reviewed and approved by someone with knowledge of the codebase. You really shouldn’t have the front end guy approving backend code.
Ai doesn’t count as a code review.
At my previous job, the policy also said you were supposed to actually check out the code and run it locally. Found a lot of bugs and issues that way.
At my current job, it’s often a rubber stamp. I’ve seen things like “that’s too many parenthesis. This won’t run” sail through. This is bad.
There should also be automated tests and checks.
A long time ago a director told me “software engineers are the most sensitive people on the planet” and I think he was right. Some people just can’t take feedback. They take something like “please sort your imports. We agreed to use isort last week” as a personal attack.