Agile – How to defend reducing the strength of code reviews

agileArchitecturecode-reviewscoding-styledevelopment-process

I have started in a new team. I have 20 years experience as a developer, and I have been in the role of a team lead in several projects.

Normally I am very much pro code reviews, but I ended up in a team that use TDD up to religious fundamentalism. Mostly this is led by a single senior resource, me being the second senior. The result is that they have implemented a code review process that requires approval for merge. Not only that it requires approval, but it also requires each individual comment being responded. All that is very nice until you start getting pull requests that can not be approved in days with tens of comments each.

In addition, when requests are done, the team does not focus on what is IMHO important (patterns, interfaces, encapsulation, layering, and method signatures), but on small details.

Example: There is a code convention that methods doing things logically connected should be in close proximity to each other. But then if you actually require that the methods must be ordered by their chronological execution, that goes a bit further than the general rule.

No one is just reasoning that if we, in the first place, did not have 50 methods in the same class, the positioning of the methods would possibly not matter that much.

Code is just full of examples where developers just go in the nitty picky details instead of focusing on the general problem.

Such a heavy code review process in my opinion creates a hostile atmosphere where a newcomer feels simply bad.

How can I justify and defend the thesis that:

  1. The merge button should be enabled by default. IMO after some iterations, if the team is conscious about quality code and if someone is non-cooperative, the team will kick him/her out.
  2. The code review should be a recommendation, but not mandatory. I believe we are all grownups and it is natural to follow good advice. Again, if someone is stupid enough to not follow, in time the team will kick him/her out.
  3. The code author should have the right to merge the code within six hours, let’s say, of the pull request creation no matter if there is approval or not.

Best Answer

How can I justify and defend the thesis that:

  1. The merge button should be enabled by default
  2. The code review should be a recommendation , but not mandatory
  3. The code author should have the right to merge the code within 6 hours lets say of the pull request creation no matter if there is aproval or not.

I don't think you should try and justify any of those, because they are almost certainly bad ideas. Code review is just about the one thing that has been consistently shown to significantly improve code quality, and you're effectively proposing to stop doing it.

Instead, put your efforts into improving your code review process:

the team does not focus on what is important(patterns, interfaces, encapsulation, layering, method signature) but on small details.

This is the problem you need to fix. Work with your team to improve their abilities, both in writing code and reviewing it. Then you'll have changed a bad process into a good one.

Oh, and never, ever use language like "religious fundamentalism" when discussing this. I hope I don't need to explain why.

Related Topic