Quais são os benefícios / desvantagens da classificação de defeitos durante uma revisão de código por pares

Há cerca de três meses, nosso grupo de engenharia lançou o Conselho de Revisão para ser usado em todas as revisões de código por pares. Hoje, tive uma discussão com uma das pessoas envolvidas nesse processo e descobri que já estamos procurando uma substituição (possivelmente algo comercial) por causa de vários recursos ausentes.

Um dos recursos aparentemente solicitados por muitas pessoas é a capacidade de classificar / categorizar cada comentário de revisão de código (isto é, é um problema de estilo, convenção de codificação, vazamento de recursos, erro lógico, falha ... qualquer coisa).

Para as equipes que praticam regularmente a revisão de código, essa categorização é uma prática comum? Você faz isto? você já fez isso no passado? Isso é bom / ruim?

Por um lado, fornece à equipe mais algumas métricas e, possivelmente, indicará áreas mais específicas nas quais os desenvolvedores podem precisar ser treinados (pelo menos esse parece ser o argumento). Existem outros benefícios? E, por outro lado, e essa é minha preocupação, é que isso desacelerará muito mais o processo de revisão de código. Como líder de equipe, fiz uma grande parte das revisões, e sempre gostei da capacidade de destacar um pedaço de código, elaborar um comentário e seguir o mais rápido possível. Embora eu não tenha tentado pessoalmente, tenho a sensação de que a expansão dessa caixa de combinação todas as vezes e a rolagem / pesquisa da categoria correta parecerão que algo está enganando você. Além disso, se começarmos a manter métricas sobre essas coisas,

Acredito que você tenha respondido amplamente à sua própria pergunta sobre os benefícios e desvantagens relativos.

As revisões de código geralmente podem acabar sendo trabalhosas e prolongadas, e tudo o que aumenta o tempo necessário para revisar o código vai piorar a situação. Na minha opinião, você deseja manter seu processo de revisão muito curto, e não se esforçar muito nos pontos mais delicados. Portanto, a chave é decidir o que é o processo de revisão que oferece valor comercial à sua equipe.

Questões de estilo são provavelmente um dos itens que eu colocaria como a mais baixa das prioridades. Certamente, manter o código limpo e formatado de maneira uniforme pode facilitar a compreensão, mas o excesso de estilo também pode resultar em enormes ineficiências durante o processo de codificação, porque se preocupar com a beleza do código afasta os pensamentos dos desenvolvedores dos problemas a serem resolvidos. Se você ainda estiver preocupado com problemas de estilo, o uso de uma ferramenta de verificação de Estilo / formatação (por exemplo: StyleCop for C #) é uma ótima maneira de deixar os problemas específicos do estilo para o último momento e tomar o processo de tomada de decisão relacionado ao estilo das mãos dos desenvolvedores, liberando seu pensamento para as coisas mais importantes. Se você não tiver esse produto no seu idioma de escolha, talvez seja possível escrever um analisador simples para verificar rapidamente seu código em busca de tais problemas,

Vazamentos de memória e outros problemas específicos de desempenho nunca devem depender do processo de revisão. Claro, se você encontrar algo que obviamente causará um grande problema, você deve destacá-lo, mas não deve ser o objetivo da revisão do código rastrear todos os pequenos problemas de memória / desempenho em seu código. É para isso que serve uma boa ferramenta de criação de perfil, e certamente valem cada centavo gasto se você conseguir localizar uma ferramenta muito boa para o idioma em que está desenvolvendo.

Os problemas de lógica são problemáticos, na melhor das hipóteses, e essas são as coisas que podem realmente consumir muito tempo valioso quando você está revisando o código. Em vez de deixar tudo isso inteiramente para a revisão de código, é para isso que seus testes de unidade devem ser usados. Sim, mesmo os testes podem estar errados; no entanto, se você desenvolver o teste primeiro, atenha-se aos princípios SRP e DRY, refatorar sem piedade e definir seus testes de unidade como um meio de validar suas especificações, você descobrirá que terá muito menos problemas relacionados à lógica. Se você testar depois de codificar, é menos provável que você lide com possíveis problemas lógicos à medida que eles surgem e mais provável que se esqueça de testar um caminho específico através do seu código.

Então, se você faz tudo como sugeri aqui, o que isso faz com que você faça na revisão de código? A resposta simples é que sua revisão de código se torna um processo bastante simples, pelo qual o codificador explica ao revisor como um determinado requisito foi capturado nos testes e como esses testes foram aplicados aos problemas resolvidos. Você tende a verificar mais detalhadamente o código e a analisar os testes mais detalhadamente, pois é nesse ponto que o maior valor comercial pode ser medido, principalmente quando esse código precisa ser mantido posteriormente. Para tornar a revisão do código de teste ainda mais simples, o uso de uma boa estrutura de teste orientada a comportamentos pode simplificar bastante a revisão, pois as especificações são capturadas no código como uma descrição quase clara em inglês sobre como o teste será executado. Verificações detalhadas são realizadas em qualquer código que suporte os testes, e se a estrutura do BDD produzir bons relatórios de texto, listando os testes em textos simples / declarações de recursos, o processo se tornará ainda mais fácil. Isso tudo resulta em um processo extremamente eficiente, que é tão valioso quanto uma revisão de código mais tradicional, mas pode ser realizado muito mais rapidamente e de maneira mais focada, ajudando você a evitar ficar atolado em trivialidades e verificações duplas. que muitas vezes levam a lugar nenhum. Essa abordagem mais enxuta significa mais tempo gasto em testes e codificação e menos tempo em processos administrativos. mas pode ser conduzido com muito mais rapidez e de maneira mais focada, ajudando a evitar esbarrar em trivialidades e verificações duplas que geralmente não levam a lugar algum. Essa abordagem mais enxuta significa mais tempo gasto em testes e codificação e menos tempo em processos administrativos. mas pode ser conduzido com muito mais rapidez e de maneira mais focada, ajudando a evitar esbarrar em trivialidades e verificações duplas que geralmente não levam a lugar algum. Essa abordagem mais enxuta significa mais tempo gasto em testes e codificação e menos tempo em processos administrativos.

Então, e essas métricas? Se todos os problemas forem tratados simplesmente como "bugs", a única métrica com a qual você realmente precisa se preocupar é o número de bugs encontrados antes e após o lançamento, ao longo do tempo, e se os bugs identificados estão tendendo em qualquer direção específica. Se você estiver usando um sistema de rastreamento de questões meio decente, todas essas informações estarão praticamente ao seu alcance e você não precisará se preocupar se o seu processo de revisão precisa identificá-las. No final do dia, sua equipe quer fazer o que é bom, que é escrever software, e não gastar muito tempo em questões administrativas que geralmente oferecem apenas algo de interesse para 1 ou 2 pessoas na empresa.

... por outro lado, e essa é a minha preocupação, é que ele desacelerará o processo de revisão de código ...

Se você deseja manter as análises leves, então consuma esforços, como investigação, correções e testes de defeitos descobertos, que não devem estar lá.

• A opção direta ( burra, se você desejar) para lidar com isso é, bem, apenas solte-a e aguarde até que os testadores encontrem e relatem o erro. A principal desvantagem dessa abordagem é que alguns problemas que são facilmente visíveis no código-fonte podem exigir muito esforço para serem descobertos no teste de caixa preta.
   
  Problemas de encadeamento e vazamentos de memória geralmente se enquadram nessa categoria. A corrida de dados que leva um minuto para ser detectada olhando para o código pode ficar lá por meses, passando em todos os testes antes de levantar a cabeça feia.

Por causa do acima, eu nunca uso e não recomendo uma opção como acima.

• Outra opção é retirar da fila o trabalho pesado - no sentido de que a fila de trabalho do revisor vai para outro lugar. Essa opção é uma que eu uso, com a qual me sinto bastante confortável.
   
  A abordagem é bastante simples. Se sinto que os problemas que encontrei podem exigir um esforço substancial para resolver, simplesmente crio um ticket no rastreador de problemas com o título "investigar e resolver problemas apontados na revisão de código <ID / URL da revisão>" e continuar através do código fazendo comentários de forma livre. Não perco tempo com coisas que serão feitas por ingresso que abri - porque ele entrará na fila de trabalho da equipe e será priorizado e tratado de maneira rotineira.
   
  Dessa forma, garanto que 1) o processo de revisão permaneça o mais leve possível e 2) os problemas descobertos não serão esquecidos.

Parece que você está procurando codestriker . Possui um recurso para classificar os problemas por modo, nível e tipo.

Para as equipes que praticam regularmente a revisão de código, essa categorização é uma prática comum?

Bem, desde que eu uso a ferramenta acima, sou forçado a fazer. O problema que vejo com isso é que, às vezes, pode ser difícil acertar, porque há muitas opções.

A classificação dos problemas fornece pouco valor à revisão do código, uma vez que o comentário é mais importante. Na verdade, como sou forçado a definir 4 campos na ferramenta acima para cada comentário, isso se tornou complicado.

Mais tarde, quando você verificar o resumo das revisões, poderá ver que a distribuição parece ter uma distribuição exclusiva. As pessoas tendem a não cometer o mesmo erro várias vezes.

Codificamos a revisão usando uma simples Causa Principal / Menor e Raiz (Requisitos / Projeto / Implementação). Manter o número de itens baixo fornece métricas úteis, sem o ônus de ser muito refinado. Raramente há uma discussão sobre uma categoria, pois há muito pouco para escolher.

Nosso rastreamento de análises é antigo, mas evoluiu ao longo de 20 anos. A ferramenta de "escolha" é o Excel, por isso já é onerosa. Estamos mudando para a Gerrit para revisões menos formais de código (Fora de "The Process", pois ele se adapta às necessidades dos desenvolvedores - com a gerrit, apenas comentamos e seguimos em frente. Não captura "métricas".

Pessoalmente, acho que as métricas são super classificadas - a revisão é sobre aprimoramento de código e, se alguém já participou de algumas, perguntará a elas as mesmas coisas que as métricas. No entanto, as métricas não podem ser discutidas. Eles também podem ser utilmente manipulados conforme necessário, e o gerenciamento não é tão sábio (às vezes sou um pouco cínico).