-1

In my team we have a policy that each code should be reviewed by at least one other developer. When the review is done the reviewer just clicks the merge button so the code ends up in development. Recently a idea came up that the reviewer should not be able to merge directly, but should pass the code review to a member of developers with permission to actually merge (2-3 people). The team size is about 8 people attempting to work according to Scrum. I would like to get some feedback on whether this is a good idea and if so in which situations.

marked as duplicate by Telastyn, Robert Harvey, David Arno, Thomas Junk, gnat Sep 9 '16 at 17:31

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

  • 3
    An idea without a justification is just an idea. What is the justification? – Robert Harvey Sep 9 '16 at 14:35
  • 1
    If the reviewer isn't a developer what they heck are they? Peer review means reviewed by your peers. A developers peers are other developers. If you are doing some other kind of review you need to tell us what it is. – candied_orange Sep 9 '16 at 14:35
  • 4
    What specific problem that you're currently facing does this solve? – Dan Pichelman Sep 9 '16 at 14:35
  • "Recently a idea came up" - came it out of thin air, or did your team actually have a problem? – Doc Brown Sep 9 '16 at 14:37
  • The thing is that I am against the idea so I am not the best person to justify it. But let me try to present the justification that was presented by others: less mistakes, 2nd opinion. The problem was that in several cases even if the code went through review some developers claimed that it should not have been merged (which might even be true, but this is not the correct solution IMO). @CandiedOrange: the reviewer is a developer. – Tomasz Grobelny Sep 11 '16 at 19:53
3

The idea of a required code review before it ends up in a shared branch is a good idea. Of course, the people who are doing the code review should be the people with the knowledge of the system under development (including the requirements, architecture, and future direction) rather than just any team members. It doesn't make much sense for a brand new hire to be assigned to do code reviews and be able to merge that in, unless they understand the larger context.

The idea of passing a code review along doesn't make that much sense to me, though. If there are a small number of more senior developers, then those people should be the ones doing the code reviews and merging, along with mentoring and coaching the less senior developers to help them get to a higher level of knowledge. As more people gain experience and knowledge, more people can be given the responsibility of conducting code reviews and merging the software.

I don't think that Scrum is going to offer you any guidance, but Lean Software Development will. Both context switching and motion are wastes that, in a lean environment, should be reduced or eliminated. In your current state, you have one person who needs to switch contexts and do work. In the proposed future state, you have two people who need to switch contexts as well as pass messages between each other.

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