Este é um resumo do curso Code Review: Best Practices by Andrejs Doroninss at Plural Sight.
Deve existir um guia de estilo padronizado e documentado para toda a equipe.
Crie um arquivo de configuração importável para cada uma de suas IDEs.
Distribua esse arquivo de configuração para os membros da equipe, de forma que a equipe use o estilo padronizado mesmo que de forma inconsciente.
A ideia é evitar discussões que refletem preferências pessoais e acabam atrasando a entrega do código em produção.
As IDEs atuais já dispõe de formatadores de código, analisadores de código e outras funcionalidades para evitar que a equipe perca tempo revisando estilo de codificação.
Uma dessas ferramentas é o sonarlint
Ao menos 2 revisores precisam aprovar o código, sendo que obrigatoriamente um dele deve ter conhecimento da lógica do sistema e seu requisitos. O outro revisor pode focar apenas na qualidade do código.
A revisão de código pode ser feita por qualquer um da equipe, não importa a sua senioridade. Um Junior pode e deve revisar o código escrito por um sênior e isso deve ser tratado como uma oportunidade para colaboração e mentoria. Esse comportamento diversifica o entendimento do código.
Tente não impor soluções que são apenas uma preferência. Melhores práticas podem ser colocadas na mesa mas outras soluções, se corretas, também devem ser aceitas.
A revisão de código deve ser algo que leve horas e não dias.
A menos que sua mudança seja muito simples, com apenas uma linha de código alterada, espera por comentários e colaboração. Esse é o mindset esperado de um sênior.
Não receber nenhum comentário em um PR atrás do outro PR não é uma situação normal. O normal é que seu PR receba comentários antes de ser aprovado.
Ao receber um comentário, não fique na defensiva achando que seu trabalho de vários dias foi questionado. Entenda que o código pertence ao time e que outra pessoa precisará entender e manter este código, portanto caso algo não esteja prontamente entendível no código que vc escreveu, ele deve ser questionado por seus colegas.
Defina com sua equipe o significado de pequeno. Por ser um limite máximo de linhas de código ou de arquivos. A equipe deve concordar com este critério.
Os benefícios são:
Para contornar o problema vc pode dividir o PR em outros pequenos e ordenados PR ou dividir a tarefa em sub-tarefas, fazendo um PR para casa divisão feita na task original.
Não deixe para comitar todo o código alterado de um só vez:
Um exemplo seria:
Um guia para commits é: commit atomic self-contained changes
O objetivo é que as ferramentas modernas de revisão permitem que a revisão seja feita por commit, e se cada commit for uma pequena atividade, fica mais fácil entender o objetivo geral da mudança.
Ao criar um PR, siga estes passos:
Se um comentário foi feito no seu PR solicitando esclarecimento sobre algo, tente melhorar a legibilidade do código, refatore ou coloque um comentário no próprio código caso o que esteja resolvendo seja muito complexo. Explicar o código no comentário do PR não trará ganho algum e provavelmente futuramente outras pessoas terão duvidas semelhantes na mesma parte do código.
É uma questão de educação e respeito ao revisor responder a todos os comentários.
Possíveis respostas são:
Não responder pode ser percebido como um esforço foi feito na revisão, mas foi simplesmente ignorado pelo autor do código.
Vc pode renomear essa variável para X?
Esse método poderia ser movido para outra classe?
Considere quebrar essa função em duas outras menores.
Evite usar termos como “você implementou isso errado.”. Ao invés de citar a pessoa, cite o código, como em: “eu não estou conseguindo entender esse código.”
Exemplos:
Você pode refatorar essa duplicação? –> Podemos evitar duplicação aqui?
Você precisa escrever teste unitário para esse código –> Esse código precisa ser coberto por teste unitário.
Observação: Essa função parece muito grande
Impacto: Isso faz com que eu tenha dificuldade para entender esse código
Solicitação: Sugiro extrair algumas partes em funções separadas e dar a elas nomes expressivos.
Outro exemplo:
Observação: Essa classe parece estar no lugar errado
Impacto: Será difícil para o time acha-la caso alguém queira usa-la
Solicitação: Considere move-la para outro pacote.
Tente aumentar o nível de detalhes:
Vc pode renomear essa variável?
Você pode tornar essa variáveis mais descritiva?
Em um código com muitos pontos de melhoria, tente focar nos dois mais importantes. Caso os demais sejam significantes o suficiente, crie uma task para refatoração subsequente.
A dica é:
O uso de labels pela equipe pode comunicar a gravidade de um problema encontrado.
Por exemplo: nitpick ou nit indica que o problema encontrado é de pequeno impacto e pode não ser aceito pelo autor do código.
Exemplos:
nit: deveria estar em camelCase
nitpick: deveria estar em camelCase
minor: deveria estar em camelCase
Os labels e suas definições podem ser acordados a nível de equipe.
Nitpick = to find faults in details that are not important
Quando encontrar algo que lhe surpreendeu de forma positiva, deixe isso claro para o autor deixando um elogio para o trabalho feito.
Faça a revisão de todo o código de uma vez. Se encontrou 5 problemas, reporte os 5 problemas, espere a correção e depois aprove o PR.
Caso um novo problema grave seja encontrado na nova revisão, reporte-o novamente. Mas não entre no jogo de revisões infinitas tentando refatorar todo o código ou mudando de ideia a cada nova revisão.
O número máximo de iterações de revisão deve ser 2, pois o nível de stress a cada iteração costuma subir
Se vc fez uma solicitação de correção, espere pela correção e aprove o código. Uma revisão deve durar horas e não dias. Termine o que vc começou.
Converse com as pessoas por chat
Converse sobre outros assuntos de interesse e hobbies
Saia para almoçar com os colegas
Se for remoto, faça encontros virtuais ocasionais, para falar de outros assuntos diferente do trabalho.
Relações de confiança fortalecem a comunicação entre as pessoas durante as revisões, uma vez que torna a relação mais leve e amigável.
Quando o revisor e o autor não entram em acordo, o melhor é tratar o problema offline, num encontro face a face, num café ou numa conversa direta um com o outro.
Existem pessoas que são dificeis de lidar:
Nestes casos algumas possíveis soluções são:
Se a solução for favorável a vc, ótimo. Se não, aceite e siga em frente