DEV Community

Discussion on: What is a good code review?

Collapse
 
heroincommunity profile image
German Chyzhov

Thanks for sharing, that is a brilliant piece.
One question is looks to me not covered here: how do you find a good code reviewers?
For instance, Alice created a PR. Who and how should select code reviewers?
I think, there could be 3 strategies:

  1. Every team member (Bob, Ann, Wagner, John) selects any PR's they want to review. Cons: the approach does not guarantee enough number and quality of reviewers, because for instance Wagner and John have never worked with that part of business/tech domain.
  2. Only team lead (Bob) is responsible for reviewing all PR's Cons: tech lead could be overwhelmed with different tasks, so review quality would suffer; he becomes a single point of failure
  3. Select reviewers based on experience in affected code components, modules, libraries and frameworks Cons: it could require a plenty of time to investigate, especially when the team is 10+ developers.

Please share your thoughts on this question.
What strategies have you used and have you met any issues with them?

Collapse
 
danimal141 profile image
Hideaki Ishii • Edited

Thank you for the comment, and I think it's a good point!

I think, there could be 3 strategies

To be honest, I select the strategies depending on the situation.

I usually select the "3" way and I don't have experience in working in a team whose members are more than 10 (basically 2 ~ 5 members).

  • If my PR is very important in the business (core DB design, API design, or something important), I assign reviewers like:
    • A tech lead (required)
    • Other members who will work in the same part in the future (optional)
  • If my PR implements a normal feature or fixes a bug
    • Someone who has worked in the same part before, or someone who is good at the part
      • For example, If I implement a front-end feature, I want to get a front-end specialist's review as much as possible (required)
    • It may be good to assign the second reviewer (e.g. junior member) to share knowledge (optional)
  • If my PR is very simple (small refactoring or something)
    • Every member
      • In this case, also, It may be good to assign a junior member to share knowledge