3 ms·

This seems like a pretty good list, I managed a team previously where they would make these mistakes in pretty much every code review - and the result of one en

This seems like a pretty good list, I managed a team previously where they would make these mistakes in pretty much every code review - and the result of one engineer being nasty during a code review was very quickly a culture where code reviews were an opportunity to settle scores and showboat rather than an actual functional process. A lot of this advice could be boiled down to "Would you make this comment on reddit, or would you say it to your colleague's face?" The only thing I would add is: >If a submission is too large to be reasonably reviewed, it is okay to let the submitter know right away. Keep moving forward. Doesn't seem right to me - or atleast the focus really should be making sure that no one submits code for review that's too big for review, and that means early feedback about a feature getting too complex and breaking it down. It's much easier to go into a problem saying "I'm going to do X, it's going to be huge, so I'll do A then B then C and submit them separately". If a submission is too big to reivew it's probably non-trivial to go back and break up so that can be quite annoying.

singron 7y ago
The number 1 reason why I actually try to make code reviews small is because the few times I made big code reviews, they just sat there. They get a few comments here and there, but the reviewers seem reluctant to review them at all, or just keep doing partial reads without the confidence to approve the whole thing. I even found the relationship non-linear. I.e. if you can break up a big change into smaller changes that have more overall code, you can still probably get those reviewed quicker. I probably would have learned the lesson faster if reviewers had just rejected the changes for being too big, or at least a "next time you could have split it this way".

3 ms·

This seems like a pretty good list, I managed a team previously where they would make these mistakes in pretty much every code review - and the result of one en

by Traster 7y ago

singron 7y ago
The number 1 reason why I actually try to make code reviews small is because the few times I made big code reviews, they just sat there. They get a few comments here and there, but the reviewers seem reluctant to review them at all, or just keep doing partial reads without the confidence to approve the whole thing. I even found the relationship non-linear. I.e. if you can break up a big change into smaller changes that have more overall code, you can still probably get those reviewed quicker. I probably would have learned the lesson faster if reviewers had just rejected the changes for being too big, or at least a "next time you could have split it this way".