5 ms·
I had a coworker who would leave nitpicks and bombshell in the same review round. Like 30 comments, 29 of which are irrelevant if you read the last one. Couldn
by progbits 2y ago
I had a coworker who would leave nitpicks and bombshell in the same review round. Like 30 comments, 29 of which are irrelevant if you read the last one.
Couldn't explain the problem to him... I just learned to read through full review from him first before starting to address anything. Otherwise he was one of the best people I worked with and the feedback was valuable.
- operator-name 2y agoIn my mind it's important to have both, as long ad the main issue is made clear and nitpicks are truly nitpicks (and thus acceptable in limited quantity to not block a change). Otherwise you'd have a "bombshell" to refactoring, then on the 2nd review pass a handful of nitpicks that you could have addressed during the initial refactor.
- sqeaky 2y agoWhat really is the alternative? Just post the big bombshell thing and not express your opinion about stuff? Often it's the expressing opinion part that's more important than the actual nits.
- progbits 2y agoIt was like: > This whole thing won't work because of X and you need to pretty much rewrite all of this. > > Rename foo to bar. > Typo. > Missing whitespace. So yeah... Just don't post the rest? I often go back to earlier comment drafts and edit / delete them when I realize deeper into the review they are no longer relevant. The review comments should not be a stream of consciousness but properly written feedback.
- aidos 2y ago> The review comments should not be a stream of consciousness but properly written feedback. My advice would be to not take it so personally and assume good intentions from your reviewers. If someone gave me feedback that caused a big course correction then I’d slap my forehead for missing it, thank them, then move on with my life. The extra feedback of other issues is a bonus. Asking them to spend more time reviewing their review is wasted effort.
- deleted 2y ago[deleted]
- dontlikeyoueith 2y ago> Just post the big bombshell thing and not express your opinion about stuff? Yes. > Often it's the expressing opinion part that's more important than the actual nits. Your opinions are not that valuable. Focus on the most valuable feedback you can give first.
- wakawaka28 2y agoIf you take the time to spell out generic complaints, it feels bad to throw that away. I think that a good compromise is to have a system that allows a summary of the review to be posted with the comments, so you can mention the important thing up front.
- sqeaky 2y agoIt isn't even about feeling bad, it's that if they write it they're going to make the same mistakes again in a little bits. If someone misspells a word habitually and you think maybe it should be spelled correctly (or whatever fix) then maybe that needs to get pointed out needs to be taken into account when they do some big rewrite.
- dontlikeyoueith 2y ago[flagged]
- manmal 2y agoThe review description (at least GitHub has this) should contain a TLDR.
- rileymat2 2y agoIs this an artifact of the tool? Promoting leaving notes as you go, for him only to find it later, with no way in the tool to reprioritize?
- progbits 2y agoThe review tool kept drafts and had a separate "submit review" step. Very easy to edit / remove comments. Most people did, he did not.