3 ms·

The worst thing during code reviews is people nit-picking my code or making comments that aren't important questions or mandatory changes. I didn't post this co

The worst thing during code reviews is people nit-picking my code or making comments that aren't important questions or mandatory changes. I didn't post this code review to get your two cents! I need to ship this code! Either give it a thumbs up, ask a relevant/important question with context, or ask me to change something that needs to be changed. Otherwise, shut up. I can handle nit-picks, compliments, curiosities, etc, but post them in Slack, not in the middle of my work. I wouldn't nit-pick you during a presentation ("this use of bullet points isn't very efficient, I would do it a different way") or critique your wardrobe at the water cooler. Don't do it to my code when I'm trying to get work done.

EdwardDiego 4y ago
I left my last job because of the org issues that led to it being stupidly hard to land PRs, because of one project lead always demanding significant changes because he didn't like X, do Y, when both were valid approaches, he just preferred Y. You'd do Y, and then he'd complain about the changes that occurred because of Y, so now do Z. And so on. I'd throw my PRs up as drafts early on, and invite his feedback as soon as possible to try to avoid this, but nope, it all came when the PR was ready for merging. In the end I decided to work in an area that this lead didn't like and didn't know about, just to get code merged without spending two weeks in review.
justahuman74 4y ago
> I didn't post this code review to get your two cents! I'd say that you actually did, that's why it wasn't a yolo-merge
j-bos 4y ago
I dunno, while I often DM people about minor improvements rather than comment directly on their code. There is value to public comments that the entire team can see. And as a bonus comments can go in with an approval, signalling their optionality.
justatdotin 4y ago
there are two voices in my head. the first is annoyed and frustrated. It says "Dave's proposal is just an irrelevant nit pick. Who cares if I make that change or not? it won't affect the product in any meaningful way." the second is relaxed and easygoing. It says "Dave's proposal is just an irrelevant nit pick. Who cares if I make that change or not? it won't affect the product in any meaningful way." I listen to the second, accept the revision, and move on.
endtime 4y ago
I have a very different perspective. I have more than a decade of experience submitting code at Google, and I recently wrote my first PR in a new language. My reviewer knew that language well and his PR was full of nitpicks, and it was great. I learned a few things, and it helped me write cleaner code the next time around.

EdwardDiego 4y ago

I left my last job because of the org issues that led to it being stupidly hard to land PRs, because of one project lead always demanding significant changes because he didn't like X, do Y, when both were valid approaches, he just preferred Y. You'd do Y, and then he'd complain about the changes that occurred because of Y, so now do Z. And so on. I'd throw my PRs up as drafts early on, and invite his feedback as soon as possible to try to avoid this, but nope, it all came when the PR was ready for merging. In the end I decided to work in an area that this lead didn't like and didn't know about, just to get code merged without spending two weeks in review.

justahuman74 4y ago

> I didn't post this code review to get your two cents! I'd say that you actually did, that's why it wasn't a yolo-merge

j-bos 4y ago

I dunno, while I often DM people about minor improvements rather than comment directly on their code. There is value to public comments that the entire team can see. And as a bonus comments can go in with an approval, signalling their optionality.

justatdotin 4y ago

there are two voices in my head. the first is annoyed and frustrated. It says "Dave's proposal is just an irrelevant nit pick. Who cares if I make that change or not? it won't affect the product in any meaningful way." the second is relaxed and easygoing. It says "Dave's proposal is just an irrelevant nit pick. Who cares if I make that change or not? it won't affect the product in any meaningful way." I listen to the second, accept the revision, and move on.

endtime 4y ago

I have a very different perspective. I have more than a decade of experience submitting code at Google, and I recently wrote my first PR in a new language. My reviewer knew that language well and his PR was full of nitpicks, and it was great. I learned a few things, and it helped me write cleaner code the next time around.