Qual é a melhor maneira de comentar em uma revisão de código?

13

Minha equipe começou a usar o cadinho / olho de peixe para iniciar revisões de código sempre que um de nós faz check-in. Há apenas três de nós, e somos encorajados a revisar o código e deixar comentários onde acharmos melhor.

Minha pergunta é: como melhor deixar um comentário em uma linha de código com a qual vejo um problema? Quero expressar meu ponto de vista sem parecer abrasivo.

Não quero parecer que estou em um cavalo alto e dizer " Venho fazendo assim ... e também não quero parecer que estou tentando ser autoritário e dizer algo como " Isso deve ser feito dessa maneira ... ", mas ainda preciso esclarecer que o que eles estão fazendo não é muito bom.

Para esclarecer: Este é realmente um bom recurso para o que eu deveria estar comentando: Uma revisão de código é subjetiva ou objetiva (quantificável)? , mas estou procurando como comentar.

stinkycheeseman
fonte
2
além de lançar nomes de FishEye e Crucible (minhas ferramentas favoritas), não vejo nada de programação específica aqui. Pode-se começar a abundância conselhos sobre coisas como que através de pesquisa na web para algo como como fornecer feedback construtivo
mosquito
@ Caleb, eu discordo, isso é mais sobre como expressar os comentários do que o outro tópico.
HLGEM
1
@HLGEM Eu diria que é exatamente disso que se trata o idiota sugerido: "Como posso sugerir com tato ...". Em geral, concentre-se na solução de problemas existentes no código em revisão, não no estilo ou na sua preferência pessoal. Explique como sua sugestão melhora o código.
Caleb
@stinkycheeseman deixe as outras pessoas saberem que fazer do seu jeito é melhor. e as pessoas da sua equipe aprenderão algo durante o processo.
Upton

Respostas:

8

Bem, costumo fazer comentários em várias áreas gerais e cada tipo pode ser tratado de maneira diferente.

Mudanças necessárias. Esses são os tipos de alterações nas quais aponto que o código não atende aos requisitos funcionais ou não funciona e deve ser corrigido antes de ser enviado à produção. Eu tendem a ser muito diretos nesses comentários. Os requisitos dizem ..., isso não faz isso. Ou isso falhará se o valor enviado for nulo (especialmente quando você sabe que esse caso ocorrerá com base nos dados que serão enviados).

Depois, há os comentários "isso funciona, mas aqui está uma maneira melhor de fazer isso". Você precisa ser mais gentil com isso e fazer mais um discurso de vendas. Eu poderia dizer que faria isso em vez disso, porque é provável que tenha melhor desempenho (geralmente reviso o código SQL, onde o desempenho é muito importante). Eu posso adicionar alguns detalhes sobre por que é uma escolha melhor, assim como eu faria ao responder uma pergunta no Stack Overflow. Eu posso ressaltar que não é necessário alterar isso para esse código específico, mas considerar a alteração na codificação futura. Basicamente, com esses tipos de comentários, estou educando pessoas com menos experiência no que pode funcionar melhor.

Depois, há os comentários "isso funciona, mas fazemos as coisas dessa maneira". Provavelmente também serão necessárias alterações. Isso incluiria comentários sobre os padrões da empresa ou a arquitetura que esperamos que eles usem. Eu referenciaria o documento padrão ou de arquitetura e diria a eles para corrigirem o padrão. O comentário seria direto, mas neutro, está faltando assim e assim ou os nomes das variáveis ​​não estão de acordo com nosso padrão de nomenclatura ou coisas semelhantes. Por exemplo, nossa arquitetura para pacotes SSIS exige que o pacote use nosso banco de dados de metadados para armazenar informações específicas sobre o pacote e requer log específico. O pacote funcionaria sem essas coisas, mas elas são necessárias por motivos da empresa (precisamos relatar a taxa de sucesso das importações, por exemplo, ou analisar os tipos de erros que recebemos.)

A única coisa que você não deseja fazer nos comentários de revisão de código é atacar alguém pessoalmente. Também pode ajudar se você encontrar algo que eles fizeram bem e apontar que foi bom. Às vezes, aprendo algo novo com uma revisão de código e, se aprendi, digo à pessoa.

HLGEM
fonte
1
Quanto ao parágrafo 3: Minha experiência é que apenas explicar uma técnica melhor raramente é boa o suficiente (a menos que seja óbvio). Você geralmente precisa reescrever o código antes que eles apreciem totalmente os benefícios e se tornem crentes. Em um sistema de revisão apenas para comentários, isso é difícil de fazer. Pode ser necessário concluir seu comentário com "Venha me ver e discutiremos". para fazer valer a pena.
Mcmcc 29/03/12
@mcmcc, esse é um argumento justo, ou você pode encaminhá-los para algum outro local do código em que uma técnica semelhante é usada. Normalmente, uso apenas os comentários para desencadear uma discussão real depois, a menos que sejam todos triviais.
HLGEM
6

Se o código seguir seus padrões de codificação, mas você fizer isso de uma maneira diferente, precisará se perguntar se o que eles fizeram está errado.

Se não é ... simplesmente não é como você faria e simplesmente não pode deixar, tente apenas perguntar 'Por que você fez dessa maneira e não dessa maneira?' Então você está conseguindo que eles se qualifiquem por que eles fizeram da maneira que fizeram sem dizer 'eu teria feito dessa maneira e você também deveria ...'

Você também pode aprender algo no processo.

Tyanna
fonte
4

Quero expressar meu ponto de vista sem parecer abrasivo.

Não confunda abrasão com ser abrasivo. Quando algo é um problema, documente-o de uma maneira que quem quer que seja corrigido possa entender. Atenha-se aos fatos e não escreva um ensaio. A saber:

  • Isso fará com que o frobnitz funcione mal quando o fooble estiver dentro de 5 queixas do fator snorgatz.

  • A convenção estabelecida para fazer isso é chamar fazzatz () com um Squidge recém-inicializado. Torne isso um método para que sempre ocorra da mesma maneira e não seja duplicado.

    Também não quero parecer que estou tentando ser autoritário e dizer algo como "Isso deve ser feito dessa maneira ...", mas ainda preciso esclarecer que o que eles estão fazendo não é muito bom. .

O objetivo de revisar o código é colocar um segundo par de olhos, geralmente mais experiente, para descobrir problemas. Se você estiver na posição de julgar o trabalho de outras pessoas e houver um motivo válido para dizer que algo não é bom, você estaria negligenciando sua responsabilidade como revisor, se não o fez.

Haverá desacordos, e essas são oportunidades para o revisor e o revisor defenderem suas posições. Se você é colega e chega a um impasse, encontre alguém mais velho para quebrar o empate.

Blrfl
fonte
+1 apenas para o fator snorgatz (bem, eu gostei do resto da resposta também)
HLGEM
3

Depende de que tipo de problema foi percebido

  • aviso de cópia ausente - um problema comum e chato, apenas um breve comentário informando o problema e seguindo em frente
  • lugares onde eu posso fazer isso de maneira diferente - geralmente tendem a fazer perguntas aqui, em vez de fazer declarações, às vezes as respostas justificam a solução original outras vezes, e então eu posso abordá-las mais explicitamente
  • locais onde há um defeito claro, por exemplo, substituição de iguais que pode ultrapassar o limite da pilha - alcançar a caneta vermelha - marcar como um defeito e ser muito explícito quanto ao motivo da quebra - também verificar outras áreas semelhantes para verificar se não houve um problema sistemático.
jk.
fonte
1

Da minha experiência:

  1. Sempre tenha o autor do código com você enquanto revê o código dele. De preferência, o código é projetado no quadro branco e vocês dois podem vê-lo claramente muito bem.

  2. Tenha uma conversa amigável. Aprecie a boa parte da codificação. Diga a ele que "isso é o melhor que eu já vi" se você vir algumas partes boas no código.

  3. Peça a ele para revisar seu código, aceitar e concordar com os pontos válidos e corrigi-lo. Respeite os comentários dele no seu código e ele respeitará automaticamente seus comentários de revisão de código.
  4. Lide com o desenvolvedor, a menos que seja muito importante ou mais tempo seja necessário para corrigir os problemas de revisão de código. Não encaminhe aos gerentes para problemas simples se houver problemas.
  5. Observe a perspectiva de "Aprendendo com outro código" em vez de apontar erros no código.
  6. Durante as sessões de revisão de código, cite os erros anteriores que você cometeu e como as revisões de código o ajudaram e evitaram grandes problemas de produção porque outro par de olhos o ajudou.
  7. Seja humilde. Mais apreciação e menos comentários para ele :) Você aprenderá muito durante a revisão do código e ele também aceitará com prazer seus comentários.
java_mouse
fonte