[matt4077]

> Have you ever did a code review, and gave feedback that was longer than doing the fix yourself, while you’re at it?

I always thought this process was sort-of intended. I feel it shows some respect to the author of the code under review to comment on it, and not to change it–especially not if that would result in changes that are then merged without first getting approval from the author. I'm having trouble finding the right words for this idea...

It's also a way to teach people about coding standards and expectations for little things, such as spelling in comments.

[kqr]

I'll provide contrast, then.

Code checked into version control is never changed. What happens is the reviewer creates a derivative work. If they use my code as a basis for their derivative work, that's a sign of respect just as much as anything else. It means that they think I actually got it mostly right.

The same argument applies if it's merged right away. Unless the reviewer(s) blatantly misunderstood something important, it's likely their fix is an improvement. And even if they did, the previous code is just a reset/rebase/oops-commit away.

That said, you should still discuss the code to iron out misunderstandings and learn things.

[merraksh]

I always thought this process was sort-of intended. I feel it shows some respect to the author of the code under review to comment on it, and not to change it–especially not if that would result in changes that are then merged without first getting approval from the author.

Agreed. It also prevents the reviewer from introducing extra bugs from knowing the code less than the author.

[alexchamberlain]

It depends what the feedback is and how you phrase it. If you want to suggest an alternative implementation, I think a PR to the author saying "hey, did you consider this?" is better than a long chain of comments trying to explain what you mean.