Revisão de código: Boas práticas para garantir a qualidade

If you want your code to be easy to write, make it easy to read. Robert C. Martin — Clean Code: A Handbook of Agile Software Craftsmanship

Assim como escrever códigos, abrir um Pull Request (aka PR) faz parte do nosso dia-a-dia. E se você quer que seu código seja revisado com a devida atenção que ele merece, seus PRs precisam ser objetivos e bem estruturados.

Separei abaixo algumas coisas que temos feito aqui no VivaReal e como isso tem ajudado a termos PRs mais fáceis de serem avaliados, o que nos deixa mais ágeis para mudanças.

Pull Request é uma maneira de documentar

Pull Requests podem ser uma excelente documentação. Um PR bem descritivo mantém o time inteirado sobre como está o processo de desenvolvimento de uma determinada feature, serve como consulta para entender o que aconteceu e o porquê de algumas decisões.

Uma boa prática é abrir o PR logo após o primeiro commit, adicionar as labels necessárias e fazer um pequeno checklist do que precisa ser feito. Para tarefas do front end também ajuda colocar prints de como a tela ficou e, para correções de bugs, um pequeno antes e depois.

Use e abuse das labels que o GitHub fornece. Elas ajudam a identificar facilmente em que estágio está o desenvolvimento, se é uma correção de bug, se é um teste a/b, se existe dependência de algum outro sistema e por ai vai. Crie seu próprio padrão de cores para ficar mais fácil de saber o que está acontecendo só de passar o olho rapidamente pela lista de PRs abertos.

Uso das tags e checklists facilitam a rápida visualização do status do PR

Mencione pessoas e referencie as issues

Não esqueça de mencionar as pessoas ou times que precisam saber sobre o pull request (cc @igorfv — O que você acha dessa solução?)

O GitHub tem uma nova feature onde você pode pedir que alguém faça um review. Adicione as pessoas que são fundamentais para o PR.

Caso seu PR esteja resolvendo uma issue, faça uma referência para ela (fixes #1234) ,(closes #1234) .

PRs precisam ser pequenos

Nada como um PR com mais de 2000 linhas para revisar em uma segunda-feira de manhã para estragar a semana de qualquer pessoa. Evitar esse tipo de situação é bem mais simples do que parece:

• Caso o sistema em que você esteja trabalhando conte com um feature toggle você pode entregar parte da solução direto pra master com a feature desligada, de bônus você evita conflitos e já valida se o que você está fazendo não vai ter algum efeito inesperado.
• Caso você não tenha um sistema de feature toggle, ou se o custo de implementar dentro de um toggle for alto, abra uma feature branch e faça pequenos PRs para ela, assim você poderá fazer merge da sua feature com a master apenas quando tudo estiver terminado.

Outras vantagem de trabalhar com PRs menores é poder validar e acompanhar a linha de pensamento do desenvolvedor. Fica muito mais fácil entender como uma nova feature funciona se você acompanhou todo o processo passo a passo.

Commits contam uma história

Um bom costume é ter commits bem descritivos, isto torna o commit melhor de ser entendido além de facilitar a revisão do código, ajuda a entender o que foi resolvido e porque foi resolvido daquela maneira.

Veja a diferença entre um commit que diz

Correção bug no menu

e um commit que diz

Correção bug no menu que impedia que o menu abrisse
- Por conta de X o código que abria o menu quebrava, foi necessário fazer XYZ para resolver esse problema
- Menu estava abrindo por cima de Y e então foi feito […]

Nada que você escreve em seu commit é perda de tempo. Ao abrir um PR em que o commit está bem escrito, o GitHub usa a primeira linha como título e o restante das linhas como descrição.

Outra coisa importante é manter seus commits limpos para que fique fácil entender o que foi feito durante o desenvolvimento. Por exemplo, quem nunca viu isso acontecer?

Corrigiu o erro mas não corrigiu direito

Você pode arrumar seus commits fazendo um squash deles através do rebase interativo, agrupando assim os dois commits de correção do bug no menu que poderiam ser um só. Na hora de fazer o merge o GitHub também fornece a opção de fazer um squash de todos os commits em apenas um.

Vou revisar um PR. E agora?

Algo que costumo seguir é a Pirâmide do Code Review. Sendo assim, a primeira coisa que precisamos validar é se o código funciona e faz o que ele deveria fazer corretamente. Só então devemos ver se o código é elegante ou fácil de ler. Essa "hierarquia" de importâncias evita ruídos desnecessários e falhas de comunicação dos PRs. Por isso é importante ter testes automatizados.

Não é uma boa idéia jogar a obrigação de garantir que o código funcione para quem está fazendo o review. Preferencialmente tenha esses testes rodando em algum CI. Se você estiver revisando um código, sempre cobre os testes para o que foi desenvolvido.

Códigos que não passam nos testes não podem ser mergeados

Como revisor também existem outros items importantes a se tomar cuidado na hora de dar seu feedback.

• Familiarize-se com o problema. A pessoa perdeu tempo escrevendo um bom PR, portando leia a descrição e entenda o que está sendo feito.
• Não seja grosseiro. Mesmo que tudo esteja ruim, sempre existe a forma educada de falar isso. Entender o contexto ajuda muito.
• Dê sugestões de alteração em vez de ordens. O que você acha? ou Você concorda? ajudam a mudar o tom.
• Caso não entenda algum ponto, pergunte.
• Reconheça os pontos positivos. Quando identificar algum código ou alguma técnica notável, não deixe de valorizar com elogios.
• Use e abuse dos emojis. Isso pode ser um problemaserá sempre mais frio e distante que um bom Isso pode ser um problema 🤔 .

Cultura reflete na qualidade

Desenvolvimento de software não é sobre escrever código para máquinas e sim para pessoas. Melhorar a escrita de reviews e PRs, e fazer com que isso faça parte da cultura do time tem um efeito bem maior do que apenas a melhora da qualidade geral do código, tem o poder de melhorar a integração entre as pessoas do time e a sua confiança sobre o código.

Essas práticas ajudaram muito o nosso time, principalmente em relação a qualidade do código. Porém cada time tem um processo e está em uma fase de maturidade diferente, por isso é importante entender em que ponto o time está para adotar os processos e mudanças de forma responsável e coerente.

Referências

• How to write the perfect pull request — https://github.com/blog/1943-how-to-write-the-perfect-pull-request
• Maslow’s pyramid of code review — http://blog.d3in.org/post/111338685456/maslows-pyramid-of-code-review