
Ask HN: Do you feel bad conducting code reviews? - sua_3000
Lately I&#x27;ve been doing most of the code reviews on my team since I have the most experience with this part of the stack.<p>I can&#x27;t help but to feel bad when code comes back 2+ times, sometimes for logic, but mostly for stylistic deficiencies.<p>How do I get myself to not feel bad, for what I imagine must be a frustrating process for the devs on my team?
======
grayhatter
I do sometimes feel bad when I do code reviews. But only when I find out after
code was merged, that I missed something when I approved the changes.

The process is frustrating on the other side, when reviews come back 2-4
times. Only because "I should be better than this!" If there's more than 2
rounds of comments on a pull/merge/patch, I'm upset that I didn't catch the
small things the first time around, and that (even though only on a very small
level, I'm wasting the time of the reviewer). Otherwise I'm happy to explain
why my way is better, or when it's not. I learn something that makes me a
better programmer, and the codebase gets better for it.

If the other devs on your team don't like code review, perhaps they need an
assignment that doesn't require code review? Because if I'm reviewing your
code and that bothers you... I'd be hard pressed to trust ANY of the code you
write.

Also: If you find you're wasting a lot of time for style, mandate a code
formatter. Then if you have CI, create a separate job for code style, you can
check that first before you even start to review code, it'll save everyone
time in the long run. Then, you'll only have to comment on logic. Style is
VERY important when doing code review, every 1/1000th of a second I spend
trying to parse the style conflicts takes away from the ability to notice an
error in the code. If you're concerned about security or performance at all,
you don't need additional overhead of having to read code through style
errors.

~~~
sua_3000
This is a great point. I don't know why we aren't running the linter via the
CI

------
27182818284
It is a frustrating process and it goes both ways.

You need to give feedback correctly both in how and why.

The developers learn to accept the feedback, but it will be a more painful
experience for them more because nobody likes their work critiqued. (This is
something that I've found trained artists are very good at, as they've been
giving and receiving critiques from both peers and instructors from the time
they were freshmen)

------
andymoe
Have your considered pairing with the other devs either full time or just once
in a while to get on the same page? At one extreme you can just not do code
reviews since you're pairing and at the other you can just do a weekly tech
retro with the team, agree to style etc and stop wasting cycles on it during
code reviews. We do both but weekly retros are magic for this type of problem.

------
sidlls
What do you mean by "stylistic deficiencies?" What fraction of the
deficiencies is the coder equivalent of semantic quibbling (e.g. naming things
or differences of opinion that have trivial impact on the code) versus
something relevant?

Seems to me this is an opportunity for you as a senior developer to exercise
some leadership: convene the team for a review and discussion about the style
guides. The focus ought to be reinforcing alignment on the purpose of the
guides, expectations about following them, and evaluating the items in them
for relevance and importance.

------
rvizconde
You shouldn't feel bad if it's part of your process and the code is not
meeting the standards agreed upon.

~~~
sua_3000
Yeah, and maybe that's part of the issue right now. There is only a tacit
agreement on using public style guides

