Earlier quoted context omitted.
Usually the feedback is not "I don't understand this" or "I find this unclear" it's "I think it would be clearer if you did it this way instead of this way". Or "if you made this change it would make it more maintainable". And if everyone followed everyone else's advice you end up with everyone writing code in their reviewers style instead of their own which doesn't seem like a gain. Not to mention code changes have…
If bugs are being introduced, that means the team's testing culture is poor. If someone asks me to make a change in code review, I know that my modifications are fine because the unit tests still pass (and the integration tests, but it's rarer to have to touch those). If I forget to manually run them the build system runs them before my code is let in. Writing in "the reviewer's style" is preferred because they are t…
> the reader's opinion is better every time.
I don't see why the reader's preference is always better. If you like designing for flexibility and I like KISS why is KISS better when you write the code, and flexibility better when I write it?
You act like our profession isn't plagued by flamewars. You see it on here every day with differences of opinion about microservices, design patterns, designing for today vs the future, KISS vs SOLID, how much testing is appropriate, should you focus more on automation vs integration vs unit tests.
When the organizations views on all these matters are aligned and documented such that the code reviews are structured that's great and you get good feedback. I've worked for 10 companies almost none of them were that aligned and organized.
Code review feedback can be very valuable, I've was the champion for code reviews 8 years ago when most organizations were not doing them. But I think the vast majority of useful code review feedback falls into one of the following categories
1. Spotted a bug, deviation from spec, or a deficiency in the spec 2. A detailed and concrete comment about maintainability (i.e. this method doesn't handle x case, or blows up on y case and that's not documented) 3. This violates a code review guideline 4. Expressing what is confusing such as complex code that needs documentation or poorly named variables 5. Performance critique
Most of the code review feedback I've seen has not fallen into these categories and instead has to do with one persons preferences vs another.
- I'd break this out into a method vs I'd inline this method - Differences in code grouping by functional vs logical cohesion. - You should make this more performant vs this isn't going to be a hotspot so build this for readability over performance