/ Processos

Revisões de Código na Prática

Este artigo faz parte de um estudo realizado pela equipe de desenvolvimento que faço parte atualmente. Nele, sintetizei (isto é, traduzi para o português, resumi e reorganizei) algumas informações que encontramos sobre maneiras eficientes de realizar a Revisão de Código (RC). No artigo anterior a este, você pode entender melhor os benefícios de uma RC. ( Link para o artigo: Sua equipe faz Revisões de Código? )

É sua vez de revisar, e agora?

Vamos lá, sem pressão! Primeiro vamos esclarecer algumas coisas… Uma revisão de código é um ponto de sincronização entre os diferentes membros da equipe e, portanto, tem o potencial de bloquear o progresso. Consequentemente, as revisões de código precisam ser imediatas (na ordem de horas, não dias), e os membros da equipe e os líderes precisam estar cientes do compromisso de tempo e priorizar o tempo de revisão de acordo.

Mesmo que pareça uma grande revisão, tente fazer a primeira análise o mais rápido possível. Seus colegas de trabalho ficarão agradecidos e sua boa vontade o ajudará a crescer mais rapidamente como revisor.

Se você acha que não pode concluir uma revisão a tempo, informe imediatamente ao autor para que ele possa encontrar outra pessoa. Nem sempre é fácil fazer RCs imediatamente, especialmente se foi alterado muito código. Se você se ver procrastinando para começar uma revisão de código, essas dicas podem ajudar você a começar mais rápido:

  • Defina um limite de tempo, como meia hora. Em sua primeira análise, gaste essa meia hora tentando entender a mudança e anotando as perguntas.

    • Se, no final do prazo, você achar que pode aprovar a alteração, aprove a alteração.

    • Se você não estiver pronto, envie ao autor quaisquer pensamentos ou perguntas que você tenha até agora, agende e se comprometa com um momento em que você pretende fazer um passe mais detalhado e aprovar ou solicitar alterações.

  • Mantenha dois repositórios separados em sua máquina, um para suas próprias alterações e outro para alterações que você está revisando. Dessa forma, compilar as alterações de seus colegas de trabalho não destruirá os artefatos de compilação associados às suas próprias alterações.

Imagine como você faria essa alteração antes de lê-la


Leia a descrição da funcionalidade primeiro e tente fazer uma lista dos arquivos que você espera alterar. Em seguida, revise os arquivos que o autor realmente alterou. Se eles são diferentes do que você esperava, descubra o porquê. Alguns arquivos podem ter sido alterados por acidente ou você pode não ter notado a importância que alguns arquivos tinham, assim pode haver uma chance de você aprender mais sobre a base de código.

Execute a aplicação e tente navegar pelas funcionalidades


A leitura do código é uma maneira bastante não natural de interagir com o código, quando o objetivo é encontrar bugs. Encontrá-los, apenas lendo, requer anos de prática.

O computador é muito melhor na execução de código do que seu cérebro. Portanto use-o a seu favor:

  • Executar o código e atingir três breakpoints pode economizar meia hora tentando entender como as peças se encaixam. Além disso, se o código seguir um ciclo de vida diferente do esperado, você provavelmente encontrou um bug ou aprendeu algo importante sobre a aplicação.

  • É provável que, ao executar a aplicação, você faça algo um pouco diferente do que o autor tentou ao testar a alteração dele. Você pode descobrir casos importantes que ele perdeu.

Visualize hierarquias de chamadas de método


Tente visualizar (ou melhor, desenhe!) quais métodos chamam quais outros métodos, ou quais objetos que o utilizam, ou então que outros objetos os auxiliam. A chave é questionar a si mesmo. A leitura não é tão boa quanto extrair algo da sua própria memória e escrever.

Menos é Mais!

“Peça a um programador para que ele revise 10 linhas de código, ele irá encontrar 10 problemas. Peça que faça o mesmo para 500 linhas e ele dirá que está tudo bem”.[1]
(@girayozil [twitter], tradução nossa)

Mudanças pequenas são preferidas em relação às maiores. Se mais de 5 arquivos foram alterados, aproximadamente, ou é uma alteração que precisa de mais de 20 minutos para ser revisada, considere realizar várias RCs independentes para essa alteração.

Por exemplo, um desenvolvedor pode enviar uma alteração que define a API para uma nova funcionalidade em termos de documentação e uma segunda alteração que adiciona implementações para essa funcionalidade.
Uma forma prática de lidar com isso é alocar uma RC para cada branch:

  1. Crie uma branch a partir da branch principal: master/nova-funcionalidade

  2. A partir dela, crie várias branches secundárias, por exemplo:
    master/nova-funcionalidade-api
    master/nova-funcionalidade-test
    Elas devem encapsular um subconjunto da funcionalidade e deverão ser revisadas individualmente em relação à branch master/nova-funcionalidade.

  3. Depois que todas as branches secundárias forem mescladas em master/nova-funcionalidade, crie uma RC para mesclar esta última na branch principal.

[1] “Ask a programmer to review 10 lines of code, they’ll find 10 issues. Ask them to do 500 lines and they’ll say it looks good.”

Revisando de fato o código


Uma revisão deve ser minuciosa o suficiente para que o revisor possa explicar a alteração em um nível razoável de detalhes para outro desenvolvedor.

Isso garante que os detalhes da base de código sejam conhecidos por mais de uma pessoa. Como revisor, é sua responsabilidade impor padrões de codificação e manter a qualidade atualizada. A única maneira de aprender é fazendo; um revisor experiente deve considerar colocar outros revisores menos experientes em suas alterações e fazer com que eles revisem primeiro. À seguir, veja uma lista de tópicos que um revisor deve se atentar em uma RC:

Objetivos:

  • Esse código cumpre o objetivo do autor? Toda alteração deve ter um motivo específico (nova funcionalidade, refatoração, correção de bug, etc). O código enviado realmente atinge esse objetivo?

  • Pergunte. Funções e classes devem existir por um motivo. Quando a razão não está clara para o revisor, isso pode ser uma indicação de que o código precisa ser reescrito ou suportado por comentários ou testes.

Implementação:

  • Você vê potencial para abstrações úteis? O código parcialmente duplicado geralmente indica que uma parte mais abstrata ou geral da funcionalidade pode ser extraída e, em seguida, reutilizada em diferentes contextos.

  • Crie configurações/dados de entrada problemáticos que quebram o código.

  • Pense nas bibliotecas ou no código de produto existente. Quando alguém reimplementa a funcionalidade existente, geralmente é porque simplesmente não sabe que já existe. Às vezes, o código ou a funcionalidade são duplicados de propósito, por exemplo, para evitar dependências. Nesses casos, um comentário de código pode esclarecer a intenção. A funcionalidade introduzida já é fornecida por uma biblioteca existente?

  • A mudança segue padrões de projeto? Bases de código consolidadas freqüentemente exibem padrões em torno de convenções de nomenclatura, decomposição de lógica de programa, definições de tipos de dados, etc. Geralmente, é desejável que as mudanças sejam implementadas de acordo com os padrões existentes.

  • A alteração adiciona dependências em tempo de compilação ou em tempo de execução (especialmente entre subprojetos)? Queremos evitar acoplamento, queremos o menor número de dependências possível. Alterações nas dependências e no sistema de compilação devem ser minuciosamente analisadas.

Legibilidade e Formatação:

  • Pense na sua experiência de leitura. Você entendeu os conceitos em um período de tempo razoável? Você conseguiu encontrar o que queria dentre os vários arquivos ou funções? Sua leitura foi comprometida por nomes, de variáveis ou métodos, inconsistentes?

  • O código adere às diretrizes de codificação e a formatação de código? O código é consistente com o projeto em termos de formatação, convenções de API, etc.?

  • Esse código tem TODOs? Os TODOs apenas se acumulam no código e se tornam obsoletos com o tempo? Peça ao autor que envie um ticket no GitHub Issues ou no JIRA e anexe o número do problema ao TODO.

Manutenibilidade:

  • Rode os testes. Se não houver testes, peça ao autor para escrever alguns. Funcionalidades verdadeiramente não testáveis são raras, enquanto implementações de funcionalidades não testadas são infelizmente comuns. Verifique os testes em si: eles estão cobrindo casos interessantes? Eles são legíveis? Pense em maneiras pelas quais esse código pode quebrar. Os padrões de formatação para testes são frequentemente diferentes do código principal, mas ainda assim são importantes.

  • Essa alteração afeta a compatibilidade com versões anteriores? Em caso afirmativo, está tudo bem mesclar a alteração neste momento ou ela deve ser enviada para uma versão posterior? Incompatibilidades podem incluir alterações no banco de dados ou no esquema, alterações na API pública, alterações no fluxo de trabalho do usuário, etc.

  • Esse código precisa de testes de integração? Às vezes, o código não pode ser testado adequadamente apenas com testes de unidade, especialmente se o código interagir com sistemas externos ou com a configuração.

  • A documentação externa foi atualizada? Se o seu projeto mantém um README, CHANGELOG ou outra documentação, ele foi atualizado para refletir as alterações? Documentação desatualizada pode ser mais confusa do que nenhuma, e será mais caro consertá-la no futuro do que atualizá-la agora.

Segurança:

  • Verifique se os endpoints da API executam a autorização e a autenticação apropriadas, consistentes com o restante da base de código. Verifique outras deficiências comuns, por exemplo, configuração fraca, entrada de usuário mal-intencionado, eventos de log ausentes, etc. Em caso de dúvida, encaminhe a RC para um especialista em segurança do aplicativo.

Comentários: Concisos e Amigáveis


Não se esqueça de elogiar um código conciso/legível/eficiente/elegante. Por outro lado, recusar ou desaprovar uma alteração não é rude. Se a mudança for redundante ou irrelevante, decline-a com uma explicação. lembre-se de ser respeitoso. Se você considerar a alteração inaceitável devido a uma ou mais falhas fatais, desaprove-a, novamente com uma explicação. Às vezes, o resultado certo de uma RC é "vamos fazer isso de uma maneira totalmente diferente" ou até mesmo "não vamos fazer isso de jeito nenhum". Em casos de desacordo entre revisor e revisado, é recomendado buscar uma terceira opinião.

Os comentários devem ser concisos e escritos em linguagem neutra. Critique o código, não o autor. Quando algo não estiver claro, peça esclarecimentos em vez de assumir ignorância. Evite pronomes possessivos, em particular em conjunto com avaliações: “meu código funcionou antes da sua mudança”, “seu método tem um bug”, etc. Evite julgamentos absolutos: “isso nunca pode funcionar”, “o resultado será sempre errado”.

Tente diferenciar entre:

  • Sugestões
    // RC (Sugestão): implemente um método de extração para melhorar a legibilidade

  • Mudanças necessárias
    // RC: Adicionar @Override

  • Pontos a discutir ou esclarecimento
    //RC: Esse é realmente o comportamento correto? então, por gentileza, adicione um comentário explicando a lógica.

Considere fornecer links ou ponteiros para explicações detalhadas de um problema.

Quando tiver terminado uma revisão de código, indique até que ponto espera que o autor responda aos seus comentários e se gostaria de rever novamente após as alterações terem sido implementadas (por exemplo, “Sinta-se à vontade para mesclar depois de responder às poucas sugestões menores.”, “Por favor, considere minhas sugestões e me avise quando eu puder dar outra olhada.”).

Respondendo à Revisões

Parte do objetivo das Revisões de Código é melhorar a alteração feita pelo autor. Portanto, não se ofenda com as sugestões do seu revisor e leve-as a sério mesmo se você não concordar. Responda a todos os comentários, mesmo que seja apenas com um simples “OK” ou “Concluído”. Explique por que você tomou certas decisões, por que algumas funções existem, etc. Se você não conseguir chegar a um acordo com o revisor, tentem uma conversa frente a frente ou procurem uma opinião externa.

As correções devem ser enviadas para a mesma branch, mas em um commit separado. A compilação de commits durante o processo de revisão torna difícil para o revisor acompanhar as alterações.

Equipes diferentes têm políticas de merge diferentes: algumas equipes permitem que apenas líderes realizem os merges, enquanto outras equipes permitem que qualquer integrante realize um merge após uma revisão de código positiva.

Sempre dê aprovação, a menos que você possa provar que existe um bug

Se você não se sente qualificado para dar aprovação, informe o autor com essas palavras e sugira uma pessoa mais adequada para revisar o código. Quando você se esquece de aprovar uma alteração, o autor não sabe se você se esqueceu ou se você acha que o código está quebrado ou se você não se importa com a espera dele. Ajude-o a sentir suas boas intenções sendo muito claro sobre qual é o próximo passo. Comunique!

Revisões de código lado a lado


Para a maioria das revisões de código, ferramentas baseadas em diff assíncronas como Reviewable, Gerrit ou GitHub são uma ótima escolha. Mudanças complexas ou revisões entre pessoas com experiência ou experiência muito diferente podem ser mais eficientes quando realizadas pessoalmente, seja na frente da mesma tela ou projetor, ou remotamente, ou através de ferramentas de compartilhamento de tela.

Exemplos de Comentários


Seguem mais exemplos que podem serem feitos em uma RC. Os comentários sugestivos das revisões são indicados por comentários no código com //RC:.

Uso de biblioteca

//RC: remova e substitua pelo MapJoiner do Google Guava
String joinAndConcatenate(Map<String, String> map, String keyValueSeparator, String keySeparator);

Gosto pessoal

int dayCount; 
//RC: Eu costumo preferir numAlgo do que algoCount. 
//você decide, mas é importante que haja consistência no projeto.

Bugs

//RC: Isso está calculando numIterations+1 iterações, é intencional?
// Se for, considere trocar a semântica de numIterations
for (int i = 0; i <= numIterations; ++i) {
...
}

Arquitetura

otherService.call(); 
//RC: Eu acho que deveríamos evitar a dependência com o OtherService. 
//Podemos discutir isso pessoalmente?

Conclusão

Agora você já é capaz de obter os benefícios das Revisões de Código!

Estude mais sobre o assunto, planeje e aplique junto de sua equipe de desenvolvimento. Veja abaixo alguns artigos interessantes:

Artigos de Referência