[perlgeek]

> Code reviews are probably the first thing a new engineer can start doing in a new role.

That really depends on what you want the code review to achieve.

Catch typos? Yes, a new engineer can do that.

Check if code fits into the existing architecture, adheres to the invariants of the code base, uses base libraries idiomatically etc? I don't think a new engineer can contribute that from the start.

> Creating metrics through issue trackers and time sheets

Which metrics? It's far too easy to create metrics that are easy to measure, rather than metrics which actually increase the business when optimized for (and developers will optimize for / game a metric when it's used to assess their performance).

[shados]

Code reviews have 2 purpose. Help the person whom's code is being reviewed improve quality/catch issues, BUT ALSO the reviewer gets familiar with the code being pushed and learn stuff. It's super important for new engineers to review code as soon as possible for the later.

[jolmg]

> BUT ALSO the reviewer gets familiar with the code being pushed and learn stuff.

The reviewer is supposed to determine what goes in or not, so how can this be an opportunity to learn from what goes in? They're the one who's supposed to determine that! Is your idea of a reviewer someone who just spectates all code going through? It's like saying people who don't know a subject should grade work from students taking a class in that subject, as it's a great opportunity to learn from what gets turned in.

[BeetleB]

>The reviewer is supposed to determine what goes in or not

This is not a given, and in fact, I would not recommend it.

For small code reviews with only one reviewer, the reviewer gives feedback and they have a conversation about it, but with only one reviewer the developer should have the final say. The problem with giving the final say to a single reviewer is you'll get plenty of pointless code changes to suit the reviewer's individual preferences. Don't waste time on what is a perfectly reasonable difference in opinion.

If you have more than one reviewer, but still a small code review, the final say should be with the reviewer(s) - if their is consensus (you could also go with voting, with its pros and cons). Having multiple reviewers prevents the likelihood of individual preferences dominating.

If it's a significant code review, the final say should be with an experienced moderator.

[tkxxx7]

> The reviewer is supposed to determine what goes in or not, so how can this be an opportunity to learn from what goes in?

You specify multiple reviewers. That's it.

[vinceguidry]

Maybe have a senior review the code afterward so the new guy can see what's being expected?

[jolmg]

So the new guy is just watching the reviewer? That sounds neither productive, nor an efficient way to get familiar with the project.

[rleigh]

That's not what they said.

You learn by doing, and also by example. So they can do the review themselves, but also see what other people pick up on. It sets expectations, and also shows how other people think.

I always find it interesting when I do the same review as one of my co-workers. We might pick up on the same things, but often they are completely different. It shows the value of having multiple perspectives on a team. Not just for review, but for design, implementation, validation, debugging, and the rest. It widens people's perspectives, serves as a means for ongoing improvement and learning, and makes the team better as a whole. We all have different backgrounds and expertise, and having people share and pass on some of their knowledge and skills is highly valuable.

[ashishuthama]

A new engineer can learn a lot from being on code reviews earlyon. Anything from the team coding style to the application architecture.

[jolmg]

A reviewer is not there to learn. If anything, they're there to teach. What good is a reviewer that can't grade others on their adherence to the coding style and application architecture of the project? They're supposed to determine what goes in or not.

[wolco]

They wouldn't be the only one reviewing or reviewing at all. It would help them understand the process if they see it in action.

[stevens32]

Reviewers not willing to learn anything during a code review is probably an easy tell for a crappy code review culture

[jolmg]

This discussion is about newcomers that know basically nothing of the project. When I say they're not there to learn, I mean their primary function as reviewers isn't to learn from what gets submitted. They can learn depending on what gets submitted, but if 90% of the time their ignorance doesn't permit them to properly determine if what's submitted to them is good or not, then what's the point of their review?