Boas diretrizes e práticas para revisão obrigatória de código [fechado]

Estamos testando a revisão obrigatória de código em cada confirmação - nada entra no mestre que não tenha sido validado por pelo menos uma pessoa e não o autor - por alguns sprints. Temos a participação de desenvolvedores e gerenciamento (que é uma situação incrível) e queremos obter alguns dos benefícios pelos quais é conhecido:

• redução óbvia de bugs
• mais consciência das mudanças que acontecem ao redor do projeto
• o efeito "Eu sei que alguém vai olhar para isso, para não ser preguiçoso" / efeito anti-cowboy
• maior consistência dentro / entre projetos

Mas estamos introduzindo algo conhecido por reduzir a velocidade e, se feito errado, pode criar uma etapa burocrática estúpida no pipeline de confirmação, que não faz nada além de levar tempo. Coisas que me preocupam:

• comentários transformando em apenas nit picking
• (hiperbolicamente) pessoas que abrem enormes problemas de arquitetura como parte de uma revisão de confirmação de duas linhas.
• Não quero influenciar respostas com outras coisas.

Embora sejamos pessoas razoáveis ​​e façamos muitas análises de si mesmas, definitivamente poderíamos usar algumas dicas de quais tipos de coisas deveríamos tentar realizar em uma sessão de revisão para realmente fazer as revisões funcionarem para nós. . Quais são algumas diretrizes e políticas que você encontrou para trabalhar?

1. Faça comentários curtos.

   É difícil manter a concentração, especialmente durante a revisão do código, por um longo tempo. Além disso, revisões longas de código podem indicar que há muito a dizer sobre o código (veja os próximos dois pontos) ou que a revisão se torna uma discussão sobre questões maiores, como a arquitetura.

   Além disso, uma revisão deve permanecer uma revisão, não uma discussão. Isso não significa que o autor do código não possa responder, mas não deve se transformar em uma longa troca de opiniões.

2. Evite revisar o código que é muito ruim.

   A revisão do código de baixa qualidade é deprimente para o revisor e o autor do código. Se um pedaço de código é terrível, uma revisão de código não é útil. Em vez disso, o autor deve ser solicitado a reescrever o código corretamente.

3. Use verificadores automatizados antes da revisão.

   Damas automáticas evitam desperdiçar um tempo precioso apontando erros que podem ser encontrados automaticamente. Por exemplo, para código C #, a execução de StyleCop, métricas de código e especialmente análise de código é uma boa oportunidade para encontrar alguns dos erros anteriores à revisão. Em seguida, a revisão de código pode ser gasta em pontos extremamente difíceis de fazer para uma máquina.

4. Escolha com cuidado as pessoas que fazem revisões.

   Duas pessoas que não conseguem se suportar não farão uma boa revisão do código da outra. O mesmo problema surge quando uma pessoa não respeita a outra (a propósito, seja o revisor ou o autor).

   Além disso, algumas pessoas simplesmente não conseguem ver seu código revisado; portanto, precisam de treinamento e preparação específicos para entender que não são criticados e que não devem vê-lo como algo negativo. Fazer uma revisão, despreparado, não ajudará, pois estará sempre na defensiva e não ouvirá nenhum crítico de seu código (tomando todas as sugestões como críticas).

5. Faça análises informais e formais.

   Ter uma lista de verificação ajuda a concentrar-se em um conjunto preciso de falhas, evitando se dedicar à seleção de itens. Esta lista de verificação pode conter pontos como:

   • Injeção SQL,
   • Pressupostos errados sobre um idioma que pode levar a erros,
   • Situações específicas que podem levar a erros, como precedência do operador. Por exemplo, em C #, var a = b ?? 0 + c ?? 0;pode parecer bom para alguém que deseja adicionar dois números anuláveis ​​com coalescência em zero, mas não é.
   • Desalocação de memória,
   • Carregamento lento (com seus dois riscos: carregar a mesma coisa mais de uma vez e sem carregá-la),
   • Estouros,
   • Estruturas de dados (com erros como uma lista simples em vez de um conjunto de hash, por exemplo),
   • Validação de entrada e programação defensiva em geral,
   • Segurança da linha,
   • etc.

   Paro a lista aqui, mas existem centenas de pontos que podem aparecer em uma lista de verificação, dependendo dos pontos fracos de um autor preciso.

6. Ajuste progressivamente a lista de verificação.

   Para permanecer construtivo e útil ao longo do tempo, as listas de verificação usadas nas revisões formais devem ser ajustadas ao longo do tempo, dependendo dos erros encontrados. Por exemplo, as primeiras revisões informais podem revelar uma certa quantidade de riscos da injeção de SQL. A verificação de injeção SQL será incluída na lista de verificação. Quando, alguns meses depois, parece que o autor agora tem muito cuidado com consultas dinâmicas versus parametrizadas, a injeção de SQL pode ser removida da lista de verificação.

Temos quase como uma lista de verificação:

• Mostre-me a descrição da tarefa.
• Acompanhe-me o resultado e mostre-o funcionando. Execute cenários diferentes (entrada inválida, etc.).
• Mostre-me os testes aprovados. Como é a cobertura do teste?
• Mostre-me o código - é aqui que procuramos ineficiências óbvias.

Funciona razoavelmente bem.

Eu acho que uma pessoa que tem poder sobre os outros seria suficiente, administrador ou moderador para cortar comentários irrelevantes, acelerar a revisão de coisas que precisam de revisão rápida. Tomador de decisão único.

Menos isso seria que essa pessoa tenha que fazer isso como tarefa principal, enquanto poderia estar fazendo outra coisa, e provavelmente você gostaria de ter a pessoa mais experiente nessa posição.

A segunda coisa é automatizar o máximo que puder!

• controlar espaços em branco
• software de controle de estilo
• compilações automatizadas antes da revisão do código
• teste automatizado antes da revisão do código

Essas coisas removerão pelo menos algumas coisas que as pessoas podem comentar sem necessidade real. Se não estiver construindo ou tiver espaços em branco à direita, não é bom o suficiente para revisão, corrija-o e solicite a revisão novamente. Se não estiver construindo ou algum teste falhar, é óbvio que não é bom o suficiente.

Muito disso depende de suas tecnologias, mas encontre o que você pode verificar automaticamente quanto mais, melhor.

Ainda não vencemos esta batalha, mas foi o que achamos útil.