
Ask HN: How does your code review process work? - ahussain
We have about 15 developers working in permeable sub-teams, each merging one PR on average per day. When a PR is created, it needs to be CRed by at least one other developer, and developers should look at each others&#x27; work while they are waiting for their PR to be looked at.
======
mkempe
Morning routine: check email, check list of open PRs that I should review,
respond to comments on my pending PRs, if any, and merge my own approved PRs.

Reviewing work by dev A: change looks good, but what is this condition for,
given this other condition elsewhere? what does the alternative really mean?
ask a couple of questions, request elaboration on a comment because we both
know why it's there since we talked about it but the maintenance team won't
have that context. Review responses to Qs in A's older PRs; looks great,
approve.

PR from dev B: oh god, why write a two-line wrapper for a standard POSIX
function and try to make a Linux-based app look like this is running on
Windows? Check comments, oh god they all say "tbd". Request that new
identifiers include some vowels, like regular English words; point out team's
coding standard we all agreed to at the beginning of the project. Review
responses to comments on B's older PRs; ah yes, he refuses to make any
improvements and adds comments full of typos; hah, B also introduced the same
typo in a previously correctly named variable; insist on improvements.

PR from dev C: that's weird, I don't get how that would work, let's compile
it. Right, it doesn't even compile. Hmmm, two new classes, without comments,
what are those for? right, they're imported but not actually used anywhere.
What problem is this PR solving? no idea, everything is named aThing,
ThingStorage, and FutureUse. Review responses on Cs older PRs; hmmm, no actual
responses, just saying "will change in another PR". Wince, shrug, write a note
to escalate to team lead at the end of the week, again.

------
twobyfour
What is it you want to know?

~~~
ahussain
Just the mechanics of how you, as a company, structure your code review
process to ensure code quality without zapping developers of the time they
need to spend writing code.

