Ask HN: Should beginners review pull requests? - gtirloni
======
scastiel
Absolutely, I see several reasons for that:

\- 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.

~~~
dyingkneepad
This!

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 :).

------
afarrell
Yes, but as a pair with a more experienced developer teaching them _how_ to
review a pull request:

\- 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...

------
wmeredith
Not to be glib, but beginner's should review everything they can.

~~~
afarrell
It is important to know where to focus. One cannot focus on "everything".

------
tiord
Imho they should.

