I agree with your comment but where is the Project in Chief?. It’s him/her who put order and talk in front of the team. And I am not talking about managers; managers are usually a person that doesn’t understand the technical part of the project.
I have worked with QA,Testers, Code Reviewer, and Auditors and the result is generally formal, a document (including email). They typically don’t talk directly with the team because some people don’t want to listen.
But I like the idea of the code reviewer talking with the team, especially in a diplomatic way. However, let’s say that the code is horrible, considerably horrible that it shows that the so-called expert, it’s not an expert. Or if one of the team is not working at all. Or if one of the team is not a team player.
It is the project in chief that it must take a decision.
For example, instead of this code:
for(int i=0;i<10;i++) {
function doSomething(i);
}
I see this code
function doSomething(1);
function doSomething(2);
function doSomething(3);
function doSomething(4);
function doSomething(5);
// etc.
The code could work (and not, it doesn’t work!), but this style raises the alarm, the dev is inexperienced, and it is up to the project in chief to “reallocate” resources and to move where he/she would do a better job.
Also, I have worked with seniors that are good but stubborn, for example, to define a class in lowercase!. I have worked with seniors that even write technical documents in first person, it was funny.