É uma boa revisão de código que usa apenas comentários de código?

16

Condições prévias

  • Equipe usa DVCS
  • O IDE suporta a análise de comentários (como TODO e etc.)
  • Ferramentas como CodeCollaborator são caras para orçamento
  • Ferramentas como o gerrit são muito complexas para instalação ou não são utilizáveis

Fluxo de trabalho

  • O autor publica em algum lugar no ramo central de recursos de recompra
  • Revisor buscá-lo e começar a revisão
  • No caso de algum revisor de perguntas / problemas, crie um comentário com um rótulo especial, como "REV". Esse rótulo NÃO deve estar no código de produção - apenas na fase de revisão:

    $somevar = 123;
    // REV Why do echo this here?
    echo $somevar;
    
  • Quando o revisor termina de postar comentários - ele apenas confirma a mensagem estúpida "comentários" e empurra para trás

  • O autor retira a ramificação do recurso e responde a comentários de maneira semelhante ou aprimora o código e empurra-o de volta
  • Quando os comentários "REV" desaparecerem, podemos pensar que essa revisão foi concluída com êxito.
  • O autor refaz interativamente a ramificação do recurso, esmaga-o para remover os "comentários" confirmados e agora está pronto para mesclar o recurso para desenvolver ou executar qualquer ação que normalmente poderia ser após uma revisão interna bem-sucedida

Suporte IDE

Eu sei que tags de comentários personalizados são possíveis no eclipse e no netbeans. Claro que também deve estar na família blablaStorm.

Exemplo de tarefa filtrada customizada de comentários no eclipse

Questões

  1. Você acha que essa metodologia é viável?
  2. Você conhece algo parecido?
  3. O que pode ser melhorado nele?
gaRex
fonte
Boa pergunta, mas não tem nada a ver com guardanapos - não é um ótimo título.
MarkJ
@ MarkJ como nomeá-lo então? Quero dizer algo artesanato, casa de campo, feito em casa. Em russo, temos a frase "на коленке". Algo não estável, ideal, não sólido, mas que funciona.
gaRex
2
@ MarkJ, gaRex: e esse novo título? Sinta-se à vontade para reverter se achar que não é apropriado para esta pergunta.
Arseni Mourzenko
@MainMa, está tudo bem
gaRex
1
O Atlassian Crucible é essencialmente gratuito para até 10 desenvolvedores. Também é a melhor ferramenta de revisão de código que já usei. A abordagem dos comentários é viável, mas pode dificultar o rastreamento do estado.
Andrew T Finnell

Respostas:

14

A ideia é realmente muito boa. Ao contrário dos fluxos de trabalho comuns, você mantém a revisão diretamente no código; portanto, tecnicamente, você não precisa de nada além do editor de texto para usar esse fluxo de trabalho. O suporte no IDE também é bom, especialmente a capacidade de exibir a lista de comentários na parte inferior.

Ainda existem algumas desvantagens:

  • Funciona bem para equipes muito pequenas, mas equipes maiores exigirão rastreamento sobre o que foi revisado, quando, por quem e com qual resultado. Embora você realmente tenha esse tipo de rastreamento (o controle de versão permite saber tudo isso), é extremamente difícil de usar e pesquisar, e muitas vezes exigirá uma pesquisa manual ou semi-manual nas revisões.

  • Não acredito que o revisor tenha feedback suficiente do revisor para saber como os pontos revisados ​​foram realmente aplicados .

    Imagine a seguinte situação. Alice está revisando pela primeira vez o código de Eric. Ela percebe que Eric, um jovem desenvolvedor, usou a sintaxe que não é a mais descritiva na linguagem de programação realmente usada.

    Alice sugere uma sintaxe alternativa e envia o código de volta para Eric. Ele reescreve o código usando esta sintaxe alternativa que acredita entender corretamente e remove o // BLAcomentário correspondente .

    Na semana seguinte, ela recebe o código para a segunda revisão. Será que ela conseguiria se lembrar de que fez essa observação durante sua primeira revisão, para ver como Eric a resolveu?

    Em um processo de revisão mais formal, Alice pôde ver imediatamente que fez uma observação e ver a diferença do código relevante, a fim de perceber que Eric não entendeu a sintaxe que ela lhe falou.

  • Pessoas ainda são pessoas. Tenho certeza de que alguns desses comentários acabarão no código de produção e outros permanecerão como lixo enquanto estiverem completamente desatualizados .

    Obviamente, o mesmo problema existe com qualquer outro comentário; por exemplo, há muitos comentários TODO em produção (incluindo o mais útil: "TODO: comente o código abaixo.") e muitos comentários que não foram atualizados quando o código correspondente foi.

    Por exemplo, o autor original do código ou o revisor pode sair, e o novo desenvolvedor não entenderia o que a revisão diz; portanto, o comentário permanecerá para sempre, esperando que alguém seja corajoso demais para apagá-lo ou realmente entender o que diz.

  • Isso não substitui uma revisão presencial (mas o mesmo problema se aplica a qualquer outra revisão mais formal que não seja realizada presencialmente).

    Especialmente, se a revisão original exigir explicações, o revisor e o revisor iniciarão uma espécie de discussão . Não apenas você se encontrará com grandes comentários de BLA, mas essas discussões também poluem o log do controle de versão .

  • Você também pode encontrar problemas menores com a sintaxe (que também existe nos comentários do TODO). Por exemplo, e se um longo comentário "// BLA" aparecer em várias linhas, e alguém decidir escrevê-lo desta maneira:

    // BLA This is a very long comment which is way beyond 80 characters, so it actually
    // fills more then one line. Since the next lines start with slashes, but not "BLA"
    // keyword, the IDE may not be able to show those lines, and display only the first one.
    
  • E, finalmente, como uma nota menor e muito pessoal: não escolha "BLA" como palavra-chave. Parece feio. ;)

Arseni Mourzenko
fonte
"para saber como os pontos revisados ​​foram realmente aplicados" Sim :) Somente honestidade do revisor. Aqui temos mais política do que ferramenta.
gaRex
1
"people are people" Sim :( Já temos milhões desses TODOs. Pode ser apenas negar ter esse recurso nos IDEs?
gaRex 4/12
"poluir o log do controle de versão". Para isso, todo o trabalho está no ramo de recursos autônomo, que posteriormente é esmagado e mesclado no desenvolvimento. Pode ser que isso possa ser feito com um simples gancho no DVCS. Mas como vejo toda a complexidade passar das ferramentas de revisão de código para os IDEs e DVCS.
gaRex
"pequenos problemas com a sintaxe" Pode ser que não é problema? Esses REVs são apenas alguns marcadores para acessá-lo rapidamente a partir do painel.
gaRex
1
@gaRex: em seguida, os membros da equipe deve usar e-mail para concordar sobre uma data rendevouz face-a-face ;-)
Doc Brown
4

Eu complementaria os comentários no código com um documento complementar. Isso resumiria as descobertas e continuaria após a remoção dos comentários. As vantagens disso seriam:

  • compacidade. Se a pessoa falhar rotineiramente em verificar se um ponteiro passado não é nulo (ou algum outro erro comum de iniciante no idioma que você está usando), o revisor pode deixar dezenas de comentários de REV nesse sentido e, no documento, pode dizer " Encontrei 37 lugares onde os ponteiros não foram verificados primeiro "sem listar todos
  • lugar para elogios. Um comentário REV this is a nice designparece meio estranho, mas minhas análises de código geralmente incluem aprovação e correções
  • interatividade. O seu comentário de amostra é "por que você fez isso?" e é muito típico. Uma abordagem somente para comentários não suporta uma conversa. A pessoa pode alterar seu código e excluir o comentário ou excluir o comentário. Mas "por que você fez isso?" e "por favor mude isso, está errado" não são a mesma coisa.
  • mantendo um registro. Um revisor posterior pode verificar se o código foi realmente alterado ou se os comentários foram removidos. Eles também podem verificar se os comentários foram "incorporados" e se o desenvolvedor não está mais cometendo os mesmos erros em uma revisão subsequente.

Eu também usaria um item de trabalho para fazer a revisão e responder à revisão e associar os checkins a ela. Isso facilita encontrar os comentários em um conjunto de alterações antigo e ver como cada um foi tratado em outro conjunto de alterações.

Kate Gregory
fonte
então precisamos de uma boa ferramenta de revisão de código. Nossa abordagem atual descrita é principalmente política, pois as pessoas devem inventar um etiquet e segui-lo.
gaRex
"compacidade" - acho que poderia ser feito com um comentário como "// REV Dude, temos 37 locais onde os ponteiros não foram verificados primeiro, incluindo este. Por favor, RTFM"
gaRex 4/12/12
"lugar para elogios" - também é possível, mas após o esmagamento será perdido :( Eu já vejo um problema - perdemos o histórico de revisões quando o esmagamento é confirmado.
gaRex 4/12
"interatividade" - o autor pode responder em outros comentários, abaixo do inicial. Assim como no estilo da Wikipedia.
gaRex
"pessoa pode ... excluir o comentário" - é um problema. 1
gaRex 4/12/12
2

Outros falaram sobre as limitações dessa abordagem. Você mencionou que ferramentas como o gerrit eram difíceis de instalar - sugiro que você dê uma olhada no phabricator (http://www.phabricator.com). Este é o sistema de revisão de código que o Facebook usa há anos e foi recentemente aberto. Não é difícil de instalar, possui excelentes fluxos de trabalho e resolve todos os problemas mencionados nos outros comentários.

Adam Hupp
fonte
Uau! Precisamos experimentá-lo em vez do nosso adorável redmine.
gaRex
"gerrit" Eu instalei - não foi tão difícil. Eu simplesmente não consigo usá-lo! E também possui interface e fluxo de trabalho feios.
gaRex
@gaRex Usamos o Reviewboard ( reviewboard.org ) em paralelo com o Redmine. Eles servem a propósitos diferentes, então tudo bem. E você pode configurar RB para link para questões Redmine ...
Johannes S.
@JohannesS. quais vcs você está usando? Você também tem algum tutorial ou wiki sobre a integração deles? É bom ter um desses.
gaRex
@gaRex Desculpe pela resposta tardia. Nós usamos o SVN, mas o RB também suporta o git (na verdade, o pessoal do RB usa o git). Sugiro dar uma olhada no site da RB. Existe uma demonstração on-line disponível (por exemplo, confira demo.reviewboard.org/r/101 )
Johannes S.