- Code review is not only to say “this is wrong” or “you should do it this way”. It’s also an opportunity for all developers (especially beginners) to say “I don’t understand why you do that”. Every piece of code should be understandable by each member of the team, whatever their experience. Maybe the beginner dev will have to fix a bug on this complex piece of code.
- Beginners often have another point of view (a more scholar, or naive one) that can help other devs seeing things differently, maybe in a better way.
- Beginner devs are as good as anyone to find potential bugs, especially about the business requirements. For instance “I think you forgot to handle this error case“.
- Finally, seeing code from more experimented devs is a good way to improve.
Code review is not finding mistakes, it’s a dialogue between a developer and the rest of their team about a piece of code. It’s a great opportunity to make everyone improve, and it works for junior or senior.
Let's add extra arguments on top:
- Creating the habit of (and scheduling time to) reviewing patches since the beginning is great. You should also encourage them to trade reviews (if you want John to review your patch, just review one of his patches and then ask for a review!). Don't create developers that are not used to allocate time for review in their schedules!
- Code written by senior devs will have to be read and modified by beginners, so you might as well included them in the review process. A lot of time they may make suggestions that leads to a code that can be better understood by newbies.
- Reviewing patches (or just being aware of them) keeps the beginners up-to-date with the recent codebase changes.
- Even if they don't understand the patches, they will get ever slightly more familiarized with the concepts involved, keywords, where each thing is, etc.
- There's always something the reviewers can do. They may not be able to assess if the locking model will lead to issues when the planets are aligned, but they may be able to check the constants against the specification, parameter misuse, off-by-one errors, etc.
- You have to put appropriate weight on who is reviewing. A Kernel patch with a Reviewed-by: Linus Torvalds is probably good to go, but the same patch with just a Reviewed-by: Donald Trump may need more reviews :).
- What things to look for
- How much scrutiny to give
- When you should actually be pulling the code to run it
Heck, _I'd_ appreciate someone teaching me these things...