O Guia Completo do Code Review Parte 2 — O papel do Autor
Fala galera! Tudo certo? A primeira parte do guia do Code Review foi uma pancada para muita gente e espero que continue sendo algo reflexivo e de grande valor para o time de todos vocês. Reforço aqui o pedido para contribuírem curtindo, comentando e compartilhando para construirmos algo ainda melhor juntos, muitos líderes e desenvolvedores se sentem sozinhos nessa jornada e no processo de desenvolvimento e não sou só eu que passo por isso, desenvolvedores desse país, uni-vos!
Parte 2— O papel do Autor
Nessa seção tenho algumas dicas e itens que passam despercebidos por todos e é interessante reforçarmos para exercitar ainda mais nossa mente a melhorar nossas entregas de software.
O autor ele pode ser o responsável principal ou simplesmente cúmplice do problema de uma entrega de software, seja uma entrega com baixa qualidade, um atraso ou uma entrega de algo errado, sem valor ou com baixo resultado obtido.
Quando eu falo cúmplice é no sentido de tomar como verdade a primeira definição que lhe foi passada sem questionamentos. Um desenvolvedor que não faz questionamentos sobre o que está fazendo é um desenvolvedor júnior que cumpre ordens!
Admiro muito as pessoas que falam e questionam independente do nível e posição que ocupam dentro das empresas, ser questionado deveria fazer parte do dia a dia de um líder para que ele evolua.
Aprendizados do Sergião
Lembro de um episódio que me marcou numa startup em que trabalhava quando questionei um dos gastos com softwares que tínhamos, quase R$ 1 milhão por ano, a empresa estava migrando quase tudo para a GCP e lá tinha um serviço similar gratuito e que mesmo na versão paga não chegaria a 10% o valor atual gasto.
Em qualquer outra empresa eu teria sido ignorado, e talvez o diretor nem teria me respondido, mas eles levaram tão a sério que meses depois montaram uma equipe para migrar toda aquela parte do sistema, esse investimento se pagaria no curto prazo, tinha um agravante a mais, esse foi o único fornecedor que não deu nenhum desconto nos tempos de pandemia, foi mais um motivo pro diretor ter me agradecido pela ideia.
Função
O que mais queremos em nossa vida como desenvolvedor é escrever código, eu já tive muito tesão por código e ainda tenho, um código bem feito é maravilhoso, mas como no sexo, as preliminares são fundamentais.
Toda empresa e time de desenvolvimento deveria ter um checklist de itens preliminares em que deveríamos avaliar antes do início da primeira tarefa daquela história. Principalmente se for algo estritamente técnico como “melhorar a performance da aplicação”, por experiência essas são as tarefas que mais demoram e geralmente não se resolvem dentro do fluxo de trabalho ou board, e ficamos totalmente perdidos em avaliar a revisão.
Antes de começar uma história ou quando for refinar tarefas dela responda algumas perguntas como, essa história se adequa a arquitetura existente? Essa tarefa pode aumentar o uso exponencialmente do produto no curto prazo? O quão crítico é esse fluxo para a empresa? Qual ponto de performance será revisto nessa tarefa? Infraestrutura, arquitetura ou código?
Aqui vale muito também o seu bom senso e entendimento da fase da empresa, nenhuma startup ou MVP precisam nascer escaláveis e na melhor e mais moderna arquitetura possível com 100% de cobertura de testes. Mas precisamos ter clareza e é um ótimo exercício para um líder e um desenvolvedor sênior se preocuparem durante um refinamento ou execução.
Mas o ponto principal desse item que chamei de função é, você sabe o que é para você executar? Está claro para todos do seu time o que vai ser entregue? Tanto do ponto de vista de infraestrutura e arquitetura como de função em si? As telas estão prontas ou houve um alinhamento mínimo de design para entrega? Os dados que serão trafegados foram previamente alinhados?
Essa é a principal função do autor do PR, sua e do seu time, se isso tudo será definido no decorrer da execução ou previamente, não me cabe julgar, se resguarde documentando e descrevendo tudo que estiver definido e o que não estiver dentro do seu PR, nos canais públicos do seu time ou no seu sistema de gerenciamento de tarefas (Trello, Jira ou ClickUp). Não é pra reescrever a Bíblia, é uma documentação minimamente plausível de produto e tecnologia, um design doc básico ou uma RFC do que será feito naquele épico, se necessário.
Tendo isso claro, dê um bom nome ao seu PR, faça uma boa descrição da sua alteração, crie nele um roteiro de testes, para o mais leigo do seu time conseguir executar aquela revisão, crie um template no Github automatizado para ajudar seu time a seguir e montar um PR decente, dê contexto da sua mudança, reforce os itens de arquitetura que usou e o porquê, envie seu PR para os revisores com uma mensagem e se coloque a disposição para explicá-lo.
Dicas de Leitura
- RFC, ADR e Postmortem por Flávio Clésio
- Uma introdução aos Design Docs por Gabriel Denoni Mendes
Qualidade da Entrega
Quando falamos em avaliar a qualidade de uma entrega, precisamos entender várias variáveis que estão em torno do nosso trabalho, o tamanho do nosso time, a maturidade e o estágio da nossa empresa, o quão crítico é a entrega e o tipo da entrega, é a correção de um bug, um refactor, uma funcionalidade nova, uma melhoria de performance, adição de testes, foi alterada a automação do CI e CD.
Se for uma correção de um bug que gerou um incidente no sistema de pagamento da sua empresa e vocês estão perdendo dinheiro, minha sugestão é pule qualquer etapa de revisão de código ou avaliação de qualidade, corrija o problema, não importa a forma, talvez resolver o problema da pior forma já estanque a sangria e salve dinheiro momentaneamente até vocês aplicarem uma correção mais elegante.
Ao realizar uma alteração, verifique se é necessário gerar algum tipo de atualização nas documentações disponíveis do projeto, vocês usam alguma documentação de arquitetura como o C4Model ou algum diagrama de infraestrutura usando o draw.io, por exemplo. Veja se é interessante mapear aquele componente no StoryBook, se vocês já tem o costume das APIs já nascerem documentadas no Swagger ou se é necessário acrescentar alguma informação no playground do GraphQL.
Essas atualizações não precisam necessariamente fazer parte desse PR principal, as vezes sua documentação não se encontra no próprio repositório, eu sugiro que faça parte e você utilize alguma IDP para tal, mas poderiam ser alterações de fora e estarem descritas no PR e são alterações muito mais tranquilas de serem aprovadas e atualizadas.
Ter isso sempre atualizado faz muita diferença em situações críticas e no entendimento do todo pelo time. Ter documentações mínimas para cenários assíncronos é fundamental, muitas vezes o autor teve problemas, o revisor também teve e não conseguiram se alinhar, ter documentações suficientes para outras pessoas assumirem é extremamente importante.
Incluir testes automatizados em sua alteração faz muita diferença, quando fazemos uma mudança de padrão de projeto em que não mudamos as entradas e não queremos alterar o resultado daquela função, mas apenas como ela se resolve fica muito difícil, complexo e trabalhoso ter que garantir novamente a entrega sem que algum detalhe saia do padrão ou pare de funcionar, nem um desenvolvedor com anos de experiência consegue prever e garantir muitos cenários de testes e diferentes variáveis de entrada, vire e mexe esquecemos os cenários em que as entradas são totalmente inesperadas e os cenários de exceção.
Por isso quanto mais crítico for essa alteração, mais testes automatizados devemos fazer para garantir os mais variados cenários possíveis, sejam eles unitários, de integração ou e2e. Eu tenho uma ótima mania que obtive com algum colega de trabalho, que dizia que um bug só é corrigido com testes automatizados.
No meu entendimento hoje e na Matchbox, nenhum projeto deveria mais nascer sem automação (CI e CD) e nós temos alguns requisitos mínimos e automatizados para que um PR esteja pronto para revisão, o autor ele só deverá entregar seu PR para um revisor, a partir do momento em que todos os requisitos do CI estejam aceitos.
Basicamente, esses requisitos vão te ajudar a entender se seu PR está seguindo as melhores práticas e se está seguro para sua aprovação. Aqui irão rodar os testes, validação de estilo, fazer o build se necessário, tem um ali em específico que irá trazer um link de preview da Vercel onde poderá se testar exclusivamente essa branch, nós usamos também o SonarCloud que tem 4 pilares (Confiabilidade, Manutenção, Segurança e Qualidade)
Ter uma ferramenta para guiar seu time minimamente sobre a qualidade de suas alterações é muito interessante e te dará um norte para saber se você não fez alguma besteira.
Para finalizar o item de qualidade, gostaria de reforçar a prática de escrever o que deve ser testado na descrição do PR e fazer muitos testes manuais, do caminho feliz e do triste (risos), ter que olhar para o código como revisor e ter que saber o que testar é horrível e desumano!
TL;DR
- Documentação (Arquitetura, Infra, Back, Front)
- Testes Automatizados
- Integração Contínua
- Mapear Cenários de Testes
Dicas de Leitura
- Backstage — O que é isso? por Rogerio Angeliski
- C4 Model por Letícia Cipriano
- Aprenda TDD na prática por Paulo Gonçalves
O tamanho do PR
Aqui eu vou citar novamente o artigo do Victor Hugo, o tamanho do pull request é mais importante do que você pensa.
Se “A qualidade das alterações está diretamente relacionada ao tamanho dos PRs” e “o Lead Time está diretamente relacionado ao tamanho dos PRs” e “é possível descobrir o tamanho ideal do PR”, podemos concluir que sim, PRs com tamanho ideal aumentam a velocidade do time e qualidade do software.
Essa é uma afirmação muito verdadeira, PRs gigantes são facilmente aprovados, pouco revisados e a aumentam muito a probabilidade de problemas.
O lead time aumenta muito com PRs grandes, a dificuldade do revisor aumenta com os riscos envolvidos e com a quantidade de testes e retestes necessários e muito mais demora para se colocar código em produção, fugindo daquilo que falamos na Parte 1 sobre trunk based.
Estamos aqui não necessariamente querendo setar algo como ideal ou “só pode executar assim ou assado”, mas querendo propor ideias e fluxos que nos ajudam a ter um processo saudável de revisão que nos ajude a ter menos problemas e colocar código em produção todos os dias.
Para termos PRs menores uma das maneiras que utilizamos muito na Matchbox são as features flags ou release toggles do papai Fowler! Use e abuse desses recursos, e divida suas tarefas em PRs de até 400 linhas ou menos, vá colocando código em produção sem afetar os clientes finais e subindo melhorias, aumentando qualidade, colocando monitoramento, incrementando recursos e fazendo o que precisa ser feito até concluir toda a história.
Como autor esteja aberto a sugestões e questionamentos, estaremos sempre discutindo sobre o código, sobre arquitetura, sobre padrão de projeto, sobre as formas de resolver problemas e não sobre sua capacidade e suas habilidades.
Claro que aqui sempre preciso reforçar alguns pontos sobre a cultura da empresa e das pessoas, mas como líderes e desenvolvedores sêniors é essencial criarmos uma cultura de feedbacks para termos um bom processo de revisão de código, e pode ser um bom passo para criarmos essa cultura termos esse processo implementado com pessoas dispostas a aplicar de forma empática essa prática.
Para fechar a Parte 2, não seja o autor que passa a tarefa para Code Review sem nem testar a tarefa, sem ver se passou por todos os requisitos de CI, que nem sabe ao certo o que está fazendo, que começa a executar sem correr atrás das definições, que não tem nenhum cuidado com os pares e não documenta nada e principalmente não faz testes automatizados em todos os PRs ou pelo menos faz PRs que aumentam a qualidade e cercam os cenários de testes.
Guia Completo do Code Review
Parte 1 — Antes da Revisão de Código
Parte 2 — O Papel do Autor
Parte 3 — Em construção
