DEV Community

loading...

Discussion on: What is your team code review process?

Collapse
josejesusochoatorres profile image
José Jesús Ochoa Torres Author

@Periklis and @charanjit; thanks for contributing to this post.

How the reviewers are assigned to the pull request?

For example in my last 3 teams we followed these approaches:

  • We have something like a table where we can see who will review our code, so when I do a PR I have to assign my ticket to this person and that is a review Q1 then when this person finish the review needs to assign to another specific teammate to review the PR one more time that should be the review Q1, just if both teammates approve it the code is merged to dev where a testing team will review it.

  • In another team, we don't have to assign it to a specific teammate, but we have a simple rule, after taking a new ticket from the backlog you have to review a Pull request thicket. In that way, we avoid having a lot of open pull requests. Teammates can review wherever thicket they want or understand.

  • In my current team we add from 2 to 5 code reviewers but we need just 2 approvals (to merge the PR). How we select our reviewers? Well, we select people who have been working with us, teammates with high skills in the main pull request topic, teammates who worked in the same than us, I mean, teammates who worked on the same files or code in the past.

In the first two approaches, it works better for me. The problem with the last one is, if you assign 5 teammates usually they will say "mm another teammate will review it"

What do you think? Is pretty similar to you?

Collapse
perigk profile image
Periklis Gkolias • Edited

Pretty much. I have seen the following three approaches (and also a "screw code review" one), all of them can work if the team is committed and knows the drawbacks.

  • Internal knowledge. I have refactored the environmental variables...who's the "guru" or the accountable person here? I will give it to them.
  • Team meeting. The dev presents their work to the team or a subset of the team. An approval or decline, should happen asap, ideally at the meeting's end.
  • Leave it hanging. Let the team know there is a pending review. Anyone can pick it up, sometimes someone not senior enough to handle it.

Any number of people can approve, they carry an equal level of responsibility though. This can turn out very complex like: "I will not approve because John has not reviewed yet" or "Screw it, lets move to it production without review".

Forem Open with the Forem app