DEV Community

Discussion on: How do you handle Code Reviews?

Collapse
 
drmonkeyninja profile image
Andy Carter

The agency I work for has built up a strong code review process over the years.

We use GitLab to create a merge request for code changes. We have pipelines set up that when an MR is created runs the code through coding standards tests. For projects that have unit tests, these are also run. If the pipelines pass then a temporary version of the site being worked on is spun up on a Continuous Deployment server. This allows reviewers to be able to test any new functionality, the MR author is expected to provide some test instructions in the MR description.

Each new MR is assigned to two developers. We're a team of about 15 developers, who are then split into 3 sub-teams. Each sub-team is responsible for particular clients and projects. When we assign reviewers we select one from the associated sub-team and one from the wider development team. We've found this helps a lot as one of the developers reviewing the code will have some familiarity with the project, whilst ensuring we still make use of the experiences from the wider team.

Assigning reviewers is all handled by an internal tool we've developed. It also ensures that we only assign people who are not off-work.

The two reviewers will test out the new functionality and if happy, review the code. Feedback is welcomed, whether that's highlighting bugs, suggestions or just asking questions. The code review process helps us catch issues before they go further down the chain, and educate the wider team by looking at how others approach problems.

Once the reviewers are happy they thumbs up the code. The second reviewer to do this hits 'merge'. Our pipelines then deploy the code to a staging server and update Asana where we manage our tasks.

Every other Friday, we get to work on non-client projects. We've used this time to really develop our internal tools for the code review process. We have a Chrome/Firefox plugin that alerts people to merge requests that need their attention (either they need to review it or respond to feedback). We also have a daily notification email that summarises what merge requests need the recipients attention.

It's interesting to read how others are handling this process as we're always open to updating our processes.

Collapse
 
xstefan profile image
Stefan Stöhr

Cross-Team reviews are a challenge I also encountered. Often the other person didn't feel "fit enough" to review the code. Having another reviewer from your own team encourages the "external" reviewer and makes them more comfortable.

Question out of curiosity: How far is your tool automated? Does it assign people automatically or does it just suggest some, but you still pick by hand?

Collapse
 
drmonkeyninja profile image
Andy Carter

It automatically assigns reviewers, although we’re open to others taking a look. We found that assigning people helped encourage those less confident in reviewing to get involved. We felt this was important as reviews aren’t just about checking the code, they’re also a learning experience.