This type of comment is the hardest to make for me as I know it means redoing a large part of the work from scratch. It would generally be avoided by having quick design reviews before starting to code something difficult or involving sweeping changes. Just highlight how you are going to do it in a few sentences.
On the other hand, letting these kind of things go through usually means you will have to deal with the outcome later, and it will be more painful.
Maybe one way to decide about making this call or not is if you find the submitted version bad enough you are ready to implement the alternative yourself, immediately.
And it happens. With junior devs, or because people are new to the codebase.
That's a good way to put it. I wonder if draft PRs/MRs could be used for that? Make a draft PR/MR with the design, do a design review, then implement the code and finally the code review.
1) obvious code smell, here’s an example using your existing code refactored and the reasons why it is a better fit here
2) you’ve done something I didn’t think of and it’s clearly better than the way I was thinking of it. Here’s why it’s better. Kudos!
Helps that I’m lead/principal in a small biz with like 4 people max writing code. So I kind of know what everyone is working on / what they’re touching with their changes.
Mileage will definitely vary in bigger teams / businesses. 100% helps that my performance isn’t tied to number of PRs reviewed etc.