[yupper32]

There are a few types of comments on code review that I see:

1. Critical mistakes

2. Style guide mistakes

3. Non-optimal design (in your opinion)

#1 Always receives a comment, obviously.

#2 Always receives a comment, but we have good automation here so it's not too much of a worry.

#3 Is where it gets interesting. A lot of the time, commenting about this category is prematurely optimizing and it's a waste of time for both the reviewer and the owner of the change. At most I try to make one overarching comment about the design.

You really have to ask yourself "Does it work? Will it cause issues down the line?" If the answers are Yes and No respectively, you should likely let it go.

The feeling of ownership over the design, especially for a junior engineer, is incredibly important for moral and the ability to get better over time. IMO it's also important to let them make small mistakes now and again so they can learn from it.

[RandallBrown]

With number 3 I usually leave a comment but it's something along the lines of "Why did you decide to do x instead of y. I think y would be better because..."

I don't necessarily want or expect them to change their code but I want to make sure they're considering multiple options and intentionally choosing that path instead of just copy and pasting something from stack overflow (as an example.)