31

I'm lead on a team with a half-dozen senior engineers. I very much believe it would benefit us greatly to do code reviews for all the standard reasons. Not necessarily every change but at least a steady stream of background reviews. So people at least see other's changes and start talking about them.

Is there a good way to introduce reviews? I sense a large reluctance from the team, because it's just one more thing to do, and conversations can get painful. I feel like reviewing every change is a non-starter, at least as a first step. I'd like people get into the rhythm and practice of doing reviews with some low frequency first before amping up the quantity.

Has anyone successfully introduced code reviews gradually? How? I've though about requiring reviews on "hot" files or libraries. Or picking at random. Or me picking "choice" must-review changes. Or is taking the plunge and doing every change the only way to go?

1
  • I didn't emphasize it enough in the question but "gradually" was the key element here. I don't think reviewing 100% of changes is at all feasible. Yet if reviewing only a portion, how does one pick the portion? Just pick "favorite changes"? Something random? Lead picks? Answers here are great but didn't really hit on the "gradual" part in my mind.
    – Philip
    Aug 27, 2016 at 3:14

5 Answers 5

18

This is not a problem of tooling or process. It's about culture. You describe a team that is comprised of people who are sensitive of criticism and protective of their own work. It's very, very common. But it is not professional.

My advice is to start leading by example. Offer your commits for review. Be open about requesting that people highlight the problems in your approach. Be receptive to feedback. Do not be defensive, but instead explore the reasons behind the feedback & agree on actions as a team. Encourage an atmosphere of open dialog. Find a champion or two in your team who are willing to do this also.

It's hard work.

38

The first step would be to walk up to someone and say "hey, would you review my code?". Be the change you want to see in your organization. If you can't find a single individual willing to do it, you won't be able to convince the whole team. If the two of you have some success, report back to the team.

Once you've found someone to review your code, ask if they mind if you review some of their code. Phrase it as a learning opportunity for you and not as an opportunity for them to improve their code.

1
  • 10
    "Hey, I'm not happy with this design, can I get a second set of eyeballs?" Is a great first step. ++
    – RubberDuck
    Aug 19, 2016 at 18:39
5

As a team lead, the most value I get out of the code review process is awareness of what is going on. I like to get a chance to see each and every changeset, even if I don't have any changes or suggestions for the developer. I call these "awareness reviews." I can turn them around in under 30 minutes so there is no bottleneck.

I would suggest you start with those. Define the code review submission process (it is pretty cut and dried if you use TFS) and get everyone engaged on submitting their changesets to you (and you only) before checkin. Tell them it is for awareness only and nobody is going to criticize their code.

After an iteration or two of awareness reviews, start inviting other members of the team to review each others code... again, for awareness only. Believe it or not, this alone can be helpful for team cohesion and code uniformity.

Once you have the whole team engaged, you will probably find that your developers just can't resist making suggestions on each others' code. It will happen naturally and organically (engineers can't help themselves!) Have the team meet to discuss this and come up with guidelines for offering constructive feedback to one another. Then set them to it. Congratulations, you are now in full code review mode!

1
  • 3
    I really like the term "awareness reviews" interesting concept. For a lead it seems natural you want awareness into what others are doing. But I think you can make the case for everyone on the team, we need to be aware what others are doing for our own benefit and theirs. Otherwise we aren't even on a team, we are just working in parallel.
    – Philip
    Aug 22, 2016 at 17:06
4

Is there a good way to introduce reviews?

There are probably several good ways, depending on your team and the benefits you hope to get from reviews, but any approach will have some common features:

  • explain what you expect: This is a new process for your team, or at least a change to the existing process, so it's only fair to let the team know why you're instituting the change, how you expect the team to benefit, and how you'll know whether it's working.

  • define the process: Walk people through the process you want them to follow for reviewing code, discussing changes, etc., so that everyone on the team knows how to proceed.

  • define the criteria: Lay out the kinds of changes that people should and shouldn't call out as needing improvement. For example, bugs and significant performance improvements are good to point out; coding standards, readability, and maintainability issues should be noted but not dwelled upon; matters of personal taste or style should be left alone.

  • discuss behavior: Point out that the goal is to improve code and foster a common understanding that will help the team write better code across the board, not to embarrass anybody, settle scores, etc. Critiques should be objective and constructive, never personal. Laying down some ground rules may help to ease qualms about having code reviewed.

  • put yourself in the hot seat first: Whether you plan to have individual reviews or group reviews, it's probably a good idea to go through the first few as a group. The first review should be of your own code so that other team members can see that the process isn't so bad and that you're willing to go through it yourself.

Start by holding a kickoff meeting to explain all of the above and address team members' concerns. Follow up with e-mail that documents the process.

I sense a large reluctance from the team, because it's just one more thing to do, and conversations can get painful.

Those are two distinct concerns. If you believe that reviews will be helpful, then you need to build time into the schedule to do them. Make sure that team members understand that reviewing is work like any other task, not something additional that they have to do while continuing to complete other tasks at the same rate.

Group review meetings should be led by a facilitator who keeps the discussion moving along, limits meeting length, and keeps things constructive. That should go a long way toward avoiding painful conversations. By the time you're ready to start individual reviews, the team will hopefully have adopted behaviors that help them keep things constructive on their own.

You should also review the review process from time to time. Get the team together every so often to discuss the process: how well it's working, how it could be improved, what practices should be abandoned, etc. Give the team ownership of the process and freedom to try new things.

-2

One way of doing it is to conduct code review sessions after each sprint, and go through everyone's changes where code is projected to a some sort of large screen so that everyone in the team can participate. Any improvements can be added as technical debt to the backlog.

This approach works, but it's not perfect.

Ultimate goal would be to use pull request to merge code back into the master or develop branch where each line of code has to be reviewed.

4
  • 4
    Sitting everyone down for hours to review all the code generated during an iteration after its over (and legitimately too late) sounds like a wonderful way to make everyone hate the idea of Code Reviews.
    – RubberDuck
    Aug 19, 2016 at 18:37
  • Well.. if it takes hours to review code generated in one sprint, then you are doing it wrong, either Sprint is 6 month, or team with 50 people, or the developers do nothing about coding or best practices. Since coding does not end after iteration it's never too late, and code always changes and it tech debt can be addressed in subsequent iterations. And doing code reviews this way allows developers who has never done it to see what to look for and so on... once it's started like that it can be gradually moved towards pull requests Aug 20, 2016 at 1:33
  • 1
    my team of 7 adds/removes/modifies several thousand lines of code over ~3 dozen commits every two weeks. A quality review of a PR takes about 15-60 min. Average PR is 3-4 commits. So, yeah. If we did it all at once as a team it would take 8 hours X 7 devs instead of 8 hours spread across the team. I resent your insinuation that we're doing something wrong. We go to prod several times a week. Do you?
    – RubberDuck
    Aug 20, 2016 at 1:42
  • We do prod once every iteration Aug 20, 2016 at 1:46

Not the answer you're looking for? Browse other questions tagged or ask your own question.