Um dos momentos mais delicados da produção de software é o code review. É durante este processo que garantimos a longevidade do código, além de grande parte da diminuição dos custos de manutenção. Se você nunca vez um code review ou analisou um Pull Request, neste post vou te mostrar 13 dicas para bons Code Review e Pull Requests.
1- Check-lists para Code Review
Houve um tempo em que eu acreditei nas check-lists. Mas como tempo percebi que, além de adicionar um passo a mais para o revisor e os autores, pouca gente seguia. E no final, muitas vezes servia apenas como fonte de discussões, tornando-se um livro de regras que separa fiéis de hereges. Nenhum check-list deveria servir a esse propósito.
Listas devem servir de norte, como uma espécie de memória. Em se tratando de código, nem sempre todos os seus pontos devem ser observados em todos os Pull Requets. Como discernir isso? Muitas vezes por experiência na área; muitas vezes através de uma conversa. A vida não pode estar escrita na pedra. Tudo – se bem conversado – pode ser alterado.
Por outro lado, entendo que cada time tem a sua própria dinâmica e seu nível de maturidade. Se você acredita que este post pode virar um check-list para servir ao seu time, vá fundo! Apenas tome o cuidado para que ele seja ferramenta e não um capítulo do livro sagrado.
2- Saiba o quê revisar
Muita coisa muda no código durante a sua evolução. As vezes fazemos uma refatoração para corrigir uma decisão de design de código – como alterar o nome de pastas, por exemplo. Outras vezes, nossas refatorações envolvem mudanças no código fonte, que até mesmo podem quebrar os testes de unidade.
Como uma boa pessoa desenvolvedora, você certamente vai analisar a base de código existente antes de iniciar qualquer codificação. Neste momento, você pode decidir o melhor momento para executar essas melhorias de design. E assim, abrir um Pull Request separado, contendo apenas essas modificações.
Sendo alterações de design apenas, o código será afetado em nada. Portanto, não é preciso revisar. As pessoas no seu time agradecem e apenas aprovam a modificação. Mas cuidado! Também não é bagunça. Ao fazer alterações no design da sua aplicação, estas devem estar muito bem alinhadas com todo time, e em especial, com a liderança técnica. Tudo isso é um cuidado para diminuir a carga cognitiva.
3 – A carga cognitiva do code review
Se você abre um pull request com 100 arquivos, você acha que a pessoa que fará a revisão, vai ler detalhadamente os 100 arquivos? Ou ainda os 30 arquivos? Fazer code review é uma atividade cansativa, em que você precisa carregar na memória (a sua, não a do computador) todo o código, e interpretar pra saber se “faz sentido”.
Por isso, o tamanho do seu pull request é inversamente proporcional a qualidade da análise dele. No nosso time defendemos que o tamanho máximo de um pull request é de 15 arquivos. Para 20 arquivos, precisamos de um bom motivo. Para mais do que 20 arquivos, o pull request é rejeitado.
Isto dito, vamos para as dicas!
4 – Code Style
Codar como um time é importante. É muito cansativo quando, durante um code review, você precisa “deduzir” o que aquele código quer dizer, ou se perguntar “Porque fizeram isso aqui diferente daquilo ali”. Grande parte do aprendizado vem da compreensão de padrões e da repetição. Não ter um code style definido quebra esses dois princípios.
E sim, cada code review é um esforço de aprendizado. A adoção de um code style e convenções de nomenclatura diminuem drasticamente o esforço nesse processo. Por este motivo, um dos pontos importantes de todo code review está em checar a adoção aos padrões de código. Ainda que todos tenhamos o nosso gosto pessoal e o nosso jeitinho de nos expressarmos pelo código, quando codamos em um time, devemos codar como apenas um.
Apesar de ser uma tarefa importante, ela pode muito bem ser automatizada com a utilização de analisadores estáticos. Procure, sempre que possível, automatizar essa etapa de validação do código, para diminuir o esforço durante o code review.
5 – Clean Code e Code Smells
Já que o código segue os padrões, é hora de verificar se ele está fácil de ser lido. Isso meio que já acontece na verificação do code style. No entanto, esta etapa de clean code vai confirmar se o código está realmente limpo e se existem alternativas que melhorem a legibilidade e manutenabilidade.
Eu recomendo a leitura e releitura constante de material sobre clean code e code smells. Uma boa fonte de referência é o site https://refactoring.guru, que além de um catálogo, também possui várias dicas e padrões de refatoração.
6 – SOLID
Com o código limpinho e cheiroso, muito provavelmente você não terá muito com o que se preocupar aqui. Mas é importantíssimo verificar o acrônimo SOLID. Um dos princípios mais violados, sem sombra de dúvida, é o princípio da responsabilidade única. Quer seja em métodos, quer seja em classes, quer seja em camadas ou serviços.
Um código que não aplica SOLID tende a dar muita dor de cabeça no futuro. Recomendo que você dê uma lida sobre o nosso conteúdo a respeito de SOLID. E que também busque outras fontes sobre esse assunto. Código orientado a objetos precisa se preocupar SEMPRE em seguir o SOLID.
7 – KISS – Keep it simple, stupid!
Ao revisar o código fonte, garanta que a implementação segue a solução mais simples possível para o problema dado. E por simplicidade, entenda aquela que é mais fácil de ser compreendida pelas outras pessoas do time, além daquela que possui menos custo de processamento. É nesta etapa que nós garantimos que não construímos uma bazuca para matar uma formiga. Sabe aquele método que recebe uma variável, que contém uma função assíncrona, que transforma um array em uma árvore binária, apenas para ordenar os números? Talvez apenas um “.orderBy” já resolva.
Mas há casos em que realmente é necessária uma solução mais complexa. Garanta que essas soluções estão o mais claras possíveis – no que diz respeito ao entendimento.
Pessoas desenvolvedoras devem ser capazes de escrever soluções simples. Não há espaço no código para vaidade.
8 – YAGNI – You aren’t gonna need it
O YAGNI serve para levantar o debate do “porque você fez isso”? As vezes, pelo excesso de zelo, codamos muito além do que realmente é necessário. E internamente nos falamos “Vai que eu preciso disso” ou “Vai que acontece esse cenário”. Mesmo quem está revisando o código pode cair neste mesmo dilema.
Em regras gerais, o nosso código precisa ser extensível. Adicionar uma nova feature não deve ser um sacrifício. Contudo, o exercício de “adivinhação” não deve encontrar espaço no desenvolvimento de software. Você pode estar jogando dinheiro fora! Uma boa forma de identificar pontos em que YAGNI é uma boa resposta, são aquelas linhas que não estão cobertas por testes de unidade.
Ao revisar o código, foque em garantir a extensibilidade e em remover as implementações prematuras.
9 – Check de design
Antes de começar o desenvolvimento, um time maduro conversa sobre as implementações e traça junto um “plano de ataque” para as demandas. Esse plano pode consistir desde a divisão do trabalho (usando critérios variados) até a estratégia de implementação: um desenho alto-nível de como seria a implementação. O check de design visa garantir que esse plano está sendo seguido. Um desvio desse plano, pode acarretar retrabalho para outras pessoas do time. Porque simplesmente ainda estão com o “plano antigo” em mente.
Outro fator importante é saber se as classes estão criadas nas pastas corretas, se o código não estaria causando alguma referência circular e coisas do gênero.
10 – Check de arquitetura
O código que você escreve para uma API não terá os mesmos requisitos de um Worker. E ambos devem ter comportamentos diferentes se estão hospedados em um IIS ou mantidos por alguma versão de Kubernetes. O check de arquitetura serve para validar se as aplicações estão executando o papel esperado delas, no contexto em que elas devem funcionar.
11 – Check de segurança
Pouca gente considera esse fato, mas o code review é um dos momentos em que garantimos a segurança da nossa aplicação. Estamos usando as bibliotecas corretas? Alguém está versionando segredos no código? Um algoritmo malicioso está sendo introduzido na base de código?
Parte dessas perguntas podem ser respondidas automaticamente, através de ferramentas como dependency track, gitleaks, checkmarks e até mesmo o sonarqube. Mas a leitura humana é importante demais, já que nenhuma dessas ferramentas consegue garantir que você não está alterando um dado durante o seu processamento.
12 – Tudo em todo lugar ao mesmo tempo
Como você pode ter deduzido, é quase impossível você reler todo o código para cada ponto dessa check list. Seria extremamente cansativo. Sendo assim, eu julgo muito importante que você assimile todo esse conteúdo o mais rápido possível. Como? Codando, fazendo e recebendo code reviews. Aproveite para ler o que os colegas também estão comentando nos Pull Requests abertos. Leia código de outros times – tem muito projeto FOSS no GitHub para ler. Com o tempo você verá que essas análises acontecem simultaneamente enquanto você lê o código.
13 – Não é sobre você. É sobre o código.
Outro ponto que não poderia deixar de dizer: Os comentários não são sobre você. São sobre o código. Quando alguém solicita um refactoring, o que está em xeque não é a sua qualidade profissional. Na verdade, é apenas uma oportunidade de melhoria que você talvez não tenha visto ou priorizado. Essa é uma das vantagens de trabalhar em time. Sem contar que, ainda que não seja sobre você, também é uma chance de aprender com os outros e criar entrosamento.
Também é importante frisar que a cordialidade – quer nas perguntas, quer nas respostas – jamais será dispensada. Mas cordialidade jamais deve ser confundida com “passar pano”. O que precisa ser mudado, deve ser mudado. Devemos cuidar apenas com a forma como o recado será transmitido.
Entre times que possuem uma intimidade maior, as vezes pode rolar uma brincadeira ou outra. Muito cuidado! Lembre-se que o ambiente é profissional. E algumas brincadeiras podem passar uma impressão ruim para quem, no futuro, ler os comentários.
Daria pra gente conversar muito mais coisas sobre esse assunto, mas aumentaria demais a carga cognitiva. Então, que tal dar uma revisada no conteúdo e deixar seus comentário?