1. Decreased value: If a person is the primary (only) developer on a subsystem, any outside code reviewer will be less informed of the subsystem he is reviewing. I don't see any value in that other than get the other reviewer up to date on a subsystem he doesn't actively develop / maintain. I guess you could argue redundancies, but if you have a big system, having every developer understand every part of the system is unrealistic.

2. Increased value: if a developer is working on a new subsystem they aren't familiar with, then the 3-6 month window restarts. The 3-6 month window isn't the time of employment, it's really the time of understanding a particular subsystem.

My point is that doing code reviews in and of itself has decreasing value after 3-6 months. The 3-6 months is the time I'm estimating for a competent developer to familiarize himself with an existing subsystem codebase.

Also, any developer that still has issues after the 3-6 window of the same subsystem is a huge red flag. Code reviews will then just be a crutch for him and the entire organization when the real solution would be to get a better developer.

Also, I'm not saying any communication about code loses value. If I want a guy to look at my code and help me work though something (usually by just listening to me explain it), that has value. I'm generally speaking of the formalized code review that must happen after any coding change. That loses value after a developer is acclimated to the subsystem he has been working on, especially if the reviewer doesn't know as much about the subsystem he is reviewing (and has no need to know). I'm a very anti-hard-process guy. A process must have strong value, otherwise it's just another speed bump.

Code reviews shouldn’t be lots of overhead. They shouldn’t devolve into style critiques. They should be the best tool you have for teaching and communicating best practices. Reviewing a more senior devs code can teach a junior a lot, and occasionally catch significant errors.

How so? Particularly if you don't do your code review right after development. That holds true for production vs. QA, but I'm not sure it does for QA vs. code review, unless your QA deployment and ticket bounce back process is overly cumbersome and you don't have time planned for it.

No i didn’t design this process.

For christ sake, no. If you want communicate new best practice, do so. And if team agrees, we will follow. Don't ambush me with new expectations during code review and don't keep my task hostage in order to push for your new best practice.

Same with teaching. Teach proactively instead of pretending juniors can solve it and then rewrite all their decisions in code rewiew.

If you have expectations, tell them in advance. It is just unfair to use code review for that.

I think the quote your replying to means existing best practices. IMO, things like "we tend to follow this naming convention" and "here's a useful language feature you may not know about" seem completely fine to say at code review time, and it's not like you can teach someone absolutely everything before they get started.

Having a reviewer hold a review hostage is a big cultural red flag, IMO. If there's a real significant disagreement, you shouldn't try to solve it via code review comments; you should move to in-person communication if possible and maybe pull other people in to take a look. I also think that in most situations, reviewers should bias toward suggestions and ideas rather than demands ("strong opinions, weakly held"), and authors should feel comfortable pushing back or proposing to merge the commit as-is and leave some change for a follow-up review.

Saying code reviews are "the best tool" for teaching and communicating best practices is maybe a stretch; I find things like pair programming to be much better for stuff like that. But I still think code reviews can be a great mechanism for teaching code-level details like code style.

And if I tell you to re-implement working code my way simply because of some personal preference, i would expect you to tell me to fuck off. Cause that’s what i’d say if you did it to me.

To give you an idea of how our code reviews are perceived, we have one junior dev, at last retrospective she spoke up about how much she valued and appreciated them.

Obviously some places don’t get it, but i think we do.

But last time there were two "seniors", each with different conflicting preferences and each insisting that your code is "bad" and "mess". (And inexperienced project manager buying their bs because they were confident). And it changed based on what blogs they read last week. Juniors were more fine with that then me, because they tended to belive seniors know what they are doing. Plus juniors worked only on one module and I did on two, so I could see how what one called crap on the meeting is demanded by the other.

And the rules were different for different people.

I think it is the ability of coder to tell "fuck off" on changes he disagree with that makes all the difference. I was comiting code I disagreed with and had to come Sunday to make changes I considered useless.

I used to be for code reviews, but now I think that it requires people with strong self awareness, knowing difference between preference/feeling and objectively betters, and many programers out there don't have that. The seniors I had trouble with were on spectrum, one a little bit other a lot. They could not tell the difference and it cause a lot of problems beyond code reviews.