Devemos tentar revisar todo o nosso código?

18

No momento, estamos modificando o processo de desenvolvimento e estou pensando se devemos tentar manter 100% de nossos commits revisados ​​por pares.

Qual é a sua experiência com relação às revisões de código?

  • Você costuma gastar "muito" tempo com eles (digamos, 1/2 horas por dia), ou apenas desliza por 5/10 minutos no máximo?
  • Você tem um tempo fixo para gastar por dia / semana / sprint / projeto?
  • Mais importante, você acha que o objetivo deve ser que 100% do código seja revisto por pares ou que 100% não é necessário?
Simeon
fonte
Tente tocar 80% do código com 20% do esforço. Invista nas melhores ferramentas automatizadas que o dinheiro pode comprar.
Job
2
As ferramentas automatizadas são ótimas, mas tornam-se inúteis, a menos que você tenha tempo e recursos para manter todos os testes atualizados.
Kieren Johnstone #

Respostas:

19

Temos uma tarefa de 'Revisão de código' em cada história. Alguém idealmente não envolvido no desenvolvimento dessa história revisará todas as alterações de código associadas a essa história. Isso funciona bem.

Muito tempo? Não depende muito de quanto código - estamos procurando erros óbvios, erros de digitação, verificação básica de sanidade lógica, exceções não detectadas etc.

É uma etapa de qualidade que encontra bugs, portanto, tem algum valor. Alocar tempo pode não ser a melhor maneira de fazê-lo - e se algo for bastante complexo, ele deverá ser revisto por código?

A propósito, é importante que outra pessoa faça a revisão do código.

Kieren Johnstone
fonte
3
"A propósito, é importante que outra pessoa faça a revisão do código ..", a pergunta implica que a mesma pessoa que escreve o código a revise? Se faz onde? Eu gostaria de consertar isso :)
Simeon
4
Sem isso não acontecer, eu estava sendo abrangente e dizendo que é importante
Kieren Johnstone
5
@Simeon, na verdade, é um equívoco assustadoramente comum de que o proprietário pode revisar seu próprio código. Ela mina toda a operação
Tom Squires
5
@ TomSquires: Exatamente. Quando você trabalha com um pedaço de código há muito tempo, pode ficar "cego" para falhas óbvias, porque o vê como o que deveria ser, e não como é. Esses problemas serão mais fáceis de identificar para alguém que nunca viu o código antes. Os escritores têm o mesmo problema e, assim como os livros não são lançados sem a revisão, o código não deve ser lançado sem a revisão. Também há outros benefícios em fazer análises de código, por exemplo, é bom para transferir conhecimento entre os membros da sua equipe.
Hammar
@hammar - obviamente tentando encontrar alguém que não escrever o código que tem o tempo para se tornar tão intimamente familiarizado com ele que eles podem prestativamente revê-lo, é um desafio
Peter Ajtai
12

Uma questão mais importante: quanto do seu código é coberto por revisões, qual é a eficácia das revisões. Se seus comentários descobrirem alguns ou nenhum problema, não será útil alcançar a cobertura completa.

Primeiro, trabalhe para tornar as suas análises mais efetivas e depois decida a cobertura.

As revisões devem ser realizadas não apenas no código, mas também no design.



Além disso, as revisões não substituem testes e ferramentas:

  • Comentários podem executar código seco
  • As revisões podem incluir análise de código
  • As análises examinam a reutilização e a legibilidade
  • As revisões podem examinar alguns aspectos da eficiência; no entanto, isso não substitui a medição real do tempo de execução e a localização dos gargalos
  • Existem ferramentas para análise de código estático
  • Existem ferramentas para testar convenções de codificação, não perca tempo com isso
  • Testes de unidade, sistema e integração
  • Os testes de unidade, sistema e testes de integração podem ser repetidos automaticamente, as revisões de código geralmente são pontuais
  • Os testes de unidade devem ter alta cobertura de código e testar os principais cenários de sucesso e condições finais; as revisões de código podem fazer isso apenas parcialmente
  • Os testes de integração testam a capacidade das unidades ou sistemas de trabalharem juntos, a revisão de código não pode substituir isso
  • O sistema testa a funcionalidade de um sistema inteiro, a revisão de código não pode substituir isso



Tente dedicar uma quantidade predefinida de tempo por mês (ou por sprint) para revisões. Selecione o código que deseja revisar no próximo slot dedicado usando uma heurística como:

  • Código que pode conter um bug não identificado que foi relatado
  • Código com esse recentemente teve bugs identificados nele
  • Código com problemas de desempenho (gargalos)
  • Código escrito por novos desenvolvedores
  • Código legado que foi atualizado recentemente por alguém que não conhecia anteriormente
  • Código em novas áreas
  • Código existente sobre o qual você deseja que novos desenvolvedores aprendam
  • Código que resolve problemas complexos
  • Código que foi identificado como complexo pelas ferramentas de análise

E lembre-se, você está revisando código (ou design ou testes) e não autores.



Eu recomendo os seguintes materiais de leitura:

Revisões seletivas do trabalho sem casa Os
segredos mais bem guardados da revisão por código de pares

Danny Varod
fonte
5

Depende.

Depende do que o seu software está fazendo:

  • Se ele controla um marcapasso eletrônico ou um ônibus espacial, então definitivamente sim.

  • Se é um protótipo descartável, provavelmente não.

Também depende de quão bem você possui recursos, qual é a experiência dos seus desenvolvedores e o que procura nas revisões de código. (Lembre-se de que o desenvolvedor médio que revisa o código de outra pessoa provavelmente notará problemas de estilo e perderá sutis erros algorítmicos ... especialmente considerando que a revisão de código é uma tarefa árdua.)

Meu conselho seria salvar seu esforço de revisão de código em códigos onde a correção é crítica e o custo de erros não detectados é alto.

Stephen C
fonte
5

Primeiro, você precisa responder a esta pergunta: Por que você revisa o código?

Com essa resposta em mãos, você pode descobrir qual código precisa ser revisado.

Algumas revisões de código realizam exatamente o que o teste faz ou teria feito. Se esse é o objetivo das suas análises, aproximar-se de 100% é uma boa ideia, se você tiver poucos testes. No entanto, permitir que as ferramentas de teste façam isso reduziria a necessidade de todo o código ser revisado.

A maioria das boas revisões parece estar focada no compartilhamento de conhecimento e no aumento dos recursos dos desenvolvedores na revisão (quem escreveu o código ou quem revisou o código). Com isso como principal motivo para as revisões, certifique-se de revisar 100% do código provavelmente é um exagero.

John Fisher
fonte
3

Em um mundo perfeito, tudo seria lido explicitamente pelo autor e revisado por pelo menos uma outra pessoa, desde especificações de requisitos a manuais do usuário até casos de teste. Mas as revisões, mesmo as simples verificações de mesa, levam tempo e custam dinheiro. Isso significa que você precisa escolher o que deve revisar e quando deve revisá-lo.

Eu recomendo priorizar as coisas a serem revisadas, escolher como você deseja revisá-las e tentar revisar o máximo possível com o nível de detalhe apropriado. A priorização pode ser baseada no tipo de artefato, como declarar que os requisitos devem ser revisados, o código de design e produção deve ser revisado e os casos de teste podem ser revisados. Dentro disso, você também pode especificar que componentes de alto risco ou alto valor recebam uma prioridade na revisão, ou talvez uma revisão mais formal.

Quanto ao tempo, tudo remonta ao nível de prioridade do componente. Houve momentos em que passei 10 a 15 minutos revisando e outras em que várias pessoas leram o código individualmente e entraram em uma sala para fazer um processo de inspeção mais formal que dura de 30 a 45 minutos (dependendo do tamanho de o módulo).

No final, é um equilíbrio entre tempo, custo, escopo e qualidade. Você não pode ter todos eles, então você precisa otimizar onde puder.

Thomas Owens
fonte
3

Como sugestão, se você planeja fazer alguma revisão, tenha algumas diretrizes compartilhadas sobre o escopo e a meta da revisão, para garantir que as revisões não causem atritos desnecessários entre os membros da equipe.

Equipes coerentes criam projetos melhores. As pessoas podem perder relacionamentos por pedidos sem sentido ou por perfeição. Sempre há uma pessoa que reclamava disso ou daquilo e incomodava os outros só porque é assim ...

NoChance
fonte
2

Reservo uma hora por dia para fazer análises por pares, mas nem sempre é necessário. Nossa base de código é compartilhada entre algumas dezenas de produtos. Nossa política é que seja aceitável uma alteração trivial no código exclusivo de um produto para fazer check-in sem revisão. Alterações mais complexas de um produto exigem uma revisão, mas pode ser tão informal quanto chamar um colega para sua mesa para analisá-lo novamente. Alterações no código compartilhado exigem uma revisão mais formal, incluindo desenvolvedores de outros produtos. Acho que nossa política atinge um equilíbrio bastante bom em comparação com outras empresas em que trabalhei.

Passo mais tempo por dia em revisões do que alguns de meus colegas com menos funções centrais, mas não considero uma quantidade razoável de tempo, porque antes da política de revisão eu poderia facilmente perder mais tempo do que rastrear erros que um desenvolvedor em outro produto introduzido.

Karl Bielefeldt
fonte
Isso é uma média? Limitar uma revisão complexa a uma hora parece estranho, e se não houver muito a revisar ... bem, não vejo como uma hora por dia seria viável?
Kieren Johnstone #
Isso não é um limite. Defino o tempo com base em quanto tempo levou, e não o contrário. Normalmente, consigo obter 2 avaliações em uma hora. Se demorar mais do que isso, seus check-ins são muito grandes, você não está recebendo uma revisão de design suficiente antes, ou suas ferramentas de revisão de código são muito complicadas. As revisões de código são uma verificação, não uma reformulação.
Karl Bielefeldt
2

Fizemos revisões 100% para o código. É muito mais barato que o teste, especialmente 100% de cobertura de código. Não gastamos muito tempo com eles, revisar mais de uma hora por dia se torna menos produtivo. (30 minutos não é muito).

Enquanto estiver zerando no processo, faça anotações. O que você achou? O que o controle de qualidade encontrou mais tarde? O que seus clientes encontraram? Por que esses insetos escaparam de você?

MSalters
fonte
7
Como a revisão é mais barata que o teste automatizado? Supondo que você escreva um teste uma vez, revise um teste uma vez e tenha um conjunto de testes estável, gaste muito menos tempo e dinheiro executando testes do que o necessário para executar qualquer tipo de revisão (durante a vida útil do projeto). Além disso, visar 100% de cobertura de código é um desperdício de recursos, o que pode ser o motivo do maior tempo / custo dos testes. Também questiono os 30 minutos nas revisões - podemos revisar um módulo por 30 minutos, mas há um tempo de preparação para ler o código inicialmente, entender seu papel no sistema e comentar sobre ele.
Thomas Owens
As alegações da @MSalters não são inéditas, embora eu também seja cético quanto a levar apenas 30 minutos. Eu li apenas sobre um local em que a inspeção foi mais econômica do que os testes de unidade automatizados e era a NASA. Nesse caso, eles acabaram com o teste de unidade por completo, porque era mais barato inspecionar manualmente o código. Obviamente, a NASA ainda tinha uma proporção de testador de 12: 1: desenvolvedor, então eles também estavam fazendo muitos outros testes ... #
Michael Michael
2
@ Thomas Owens: testes de unidade encontram bugs simples. Os erros caros são aqueles em que várias unidades se combinam de maneiras inesperadas. Outro tipo de erro é a falta de esquinas. Um desenvolvedor que perdeu um caso também não fará um teste de unidade, mas um segundo par de olhos o identificará.
MSalters
@MSalters É por isso que você escreve testes de integração e sistema automatizados, bem como testes de unidade. Realmente, os únicos testes que podem precisar ser executados manualmente são os testes de aceitação. E a revisão dos testes na criação ajudaria a garantir que os casos mais críticos sejam testados.
Thomas Owens
2

Faça revisões regulares do código, principalmente para formar equipes e compartilhar idéias sobre a implementação. Você pode aprender muito com seus colegas de trabalho dessa maneira.

codificador
fonte
Isso indica apenas alguns benefícios. Você acha que encontrar erros é importante? Sendo assim, quanto?
JeffO
É claro que encontrar erros é importante, mas o benefício maior é o conhecimento de longo prazo obtido com as revisões de código. Talvez um bug foi criado por uma má abordagem que pode ser evitada no futuro
codificador
2

Exigimos uma revisão de código por pares para cada check-in. Se nenhum colega estiver disponível, providenciaremos uma revisão após o check-in. O revisor é anotado no comentário do check-in do controle de origem.

Eles não demoram muito tempo e, como são feitos entre colegas, não há aspecto tóxico de criança adulta para eles.

Jim In Texas
fonte
2

É necessário IMO Review Code. Você está 99,999 ...% do tempo nem sempre está certo, então você precisa ter certeza de que está correto. Eu tenho um horário definido? Não. Mas dedico um tempo para verificar meu código. Normalmente, tenho um colega que faz o mesmo.

Dinâmico
fonte
1

As revisões de código podem parecer assustadoras, mas são uma ferramenta valiosa quando conduzidas adequadamente. Eles serão sua primeira linha de defesa contra erros de design e implementação. Se você não estiver realizando revisões de código em todos os recursos que você implementou, inicie o mais rápido possível.

Quanto tempo gasta em revisões por pares, uma boa prática é deixar de 5 a 10% do tempo total estimado de desenvolvimento para conduzir e responder à revisão de código.

Temos um whitepaper sobre a realização de análises de código eficazes que podem ajudar você a começar com o pé direito. É um guia passo a passo e discute as armadilhas comuns que você pode enfrentar e como resolvê-las. Você pode fazer o download em nosso site.

Katie B.
fonte