[JamesBarney]

I've found PRs are usually a waste of time because people focus on such trivial bullshit because they're easy to find.

98% of the review comments I receive hark back to some handwavy explanation about maintainability. But never in any way that could actually cause a bug.

when I run teams my rules with PRs are if it's not a bug, and it's not against the code review guidelines leave it. It's usually not worth the back and forth unless there is a clear mentor/mentee relationship.

[Arainach]

This is exactly backwards.

One person writes the code. Many people read the code. If something is unclear or feels wrong to the reader, they get priority. Reviewers should leave all sorts of feedback; the response of the author should be to just implement the feedback unless they can clearly articulate why doing so is not a good idea. You're right that "it's not worth the back and forth", but completely wrong on who should be the one keeping their mouth shut.

[JamesBarney]

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 a cost, very few devs spend as much time testing and thinking about their code when making code changes. So many times you are trading more thought out, better tested code for less tested less thought out code. Because of this I've seen countless bugs introduced from code changes related to trivial PR requests.

[Arainach]

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 the reader. For most things that are "style" there should be explicit style guides; for the things that aren't, the reader's opinion is better every time.

If your team isn't getting useful code review feedback, that's also a team culture problem and your senior eng need to step up and lead by example.

"I don't understand this" is important feedback. At a minimum, more comments are needed, but it probably also signals that variable or method names need improvement.

My experience (15+ years, companies of all shapes and sizes from college shops to FAANG) is that the kind of engineer who feels that code review feedback isn't worth listening or is a waste of time to is detrimental to the team in both the short and long term.

[JamesBarney]

Very few places have enough testing that code modifications are not correlated with bugs. Based on your comment I think you maybe you're working at places that aren't very representative of the industry as a whole.

> 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