|
|
|
|
|
by JamesBarney
1301 days ago
|
|
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. |
|
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.