Uma prática bem comum de times de desenvolvimento é a revisão de código. Compartilharei alguns princípios que mais me foram úteis na carreira para prover suavidade, pragmatismo e utilidade a esse momento, tornando-o até gostoso. 😀
O contexto específico aqui é revisão via Pull Requests no GitHub em repositório de código fechado. E nada escrito aqui é regra, todos os princípios exigem que as pessoas do time leiam o momento e decidam o que fazer.
1. O objetivo de um pull request é que colegas tomem ciência e concordem com a alteração
Por quê? 🤔
Enquanto revisor(a), sua bússola deve estar apontada para entender as novidades que estão entrando no código e ajudar colegas a pensar em detalhes despercebidos. Perceba a palavra “ajudar”, ou seja, não se trata sobre se esforçar em achar defeitos, nem mostrar que você sabe mais, muito menos forçar o código a parecer como se tivesse sido escrito por você.
Em segundo plano, a ideia também é de aprender “truques novos” — especialmente as pessoas menos experientes. Ler código dos outros é uma ótima forma de também aprender a programar melhor.
Mas quando? 🕰️
Reserve horas específicas do dia para code review. É parte da rotina, tanto quanto almoçar ou fazer uma reunião diária. Guarde momentos para proativamente ir atrás de revisar o código de seus pares.
Uma menção honrosa aqui, é que há times que nunca fazem explicitamente esse passo de code review, pois trocam por sessões de pair programming. Se você parar pra pensar, fazer pair cumpre todos os objetivos acima implicitamente, aí o PR vira tipo um organizador de commits pra passar o olho rápido e facilitar a leitura ao mesclar na main branch, com bem menos formalidade.
O que evitar 🚫
Infelizmente, há casos em que as pessoas encarregadas da revisão tratam como uma tarefa “extra” a ser feita “se der tempo”. Isso costuma ser reflexo de uma cultura de engenharia mais individualista, em que a gestão direta falha em focar nos resultados do time e presta atenção apenas se todos estão ocupados digitando algo. Tarefa em revisão não é tarefa feita, falta chegar aos usuários ainda.
Um outro problema ainda pior, é despertar a Síndrome de Pequeno Poder para “avaliar” e “educar” colegas à força em tom professoral. É o(a) revisor(a) tentando re-programar o código à própria imagem via comentários. Isso apenas atrapalha, lembre que o motivo pelo qual o código tá sendo feito é pra resolver um problema, não é uma obra de arte para ser lapidada até parecer bonita. A posição de revisor(a) é de entender e tentar ajudar, não sequestrar para si o PR.
Abrir PRs (assim como revisar) deve ser uma tarefa (quase) diária. As tarefas de um time devem ser organizadas de forma que código possa estar entrando todos os dias, ou algo bem próximo disso. Alguém passar uma ou mais semanas inteiras para abrir um único PR quase que certamente vai resultar em diffs gigantes que ninguém consegue revisar, às vezes até acumulando muitas pequenas gambiarras. Pior ainda quando um PR grande depende de outro também grande, fica um caos de conflitos e branches.
2. Não personifique um linter, tenha objetividade
Reforçando a integração contínua (CI) 🤖
Este princípio já é muito falado pelo povo, mas é sempre bom reforçar. Toda linguagem hoje em dia tem alguma tecnologia que analisa o código pelo texto escrito e força a maioria de práticas comuns, os linters e code formatters. Faça questão de instalar essas coisas em seu repositório (automatizando cada PR via GitHub Actions) ou até no editor (como extensões de VS Code) para evitar discussões sobre sexo dos anjos distraindo o time.
Veja. Se os comentários durante suas code reviews estão muito parecidos com code linting, considere um mau sinal. Porque a atenção humana é um recurso limitado e frágil. Um time que se debruça sobre uma linha de código para discutir trivialidades de como ela deveria estar mais bonita, muito possivelmente está deixando passar despercebido um problema grave numa linha logo abaixo dela.
Naturalmente, isso não é desculpa para ignorar completamente questões de legibilidade e manutenibilidade. Apenas evite de “texto bonito” virar o foco da revisão.
Linha tênue entre objetividade e subjetividade 💅🏽
Por outro lado, também não precisa de ter medo de tecer revisões só porque você ficou em dúvida demais se elas eram besteira da sua cabeça ou de fato problema de legibilidade. Como, por exemplo, adotar mais early returns ao invés de if-else em códigos procedurais.
Nestes casos, onde a linha é tênue, relembre que a função da revisão é ajudar e não sequestrar a autoria do PR. Isso quer dizer que a própria pessoa que escreveu o código em primeiro lugar pode e deve decidir se aquilo é “besteira” ou não.
Exemplo mais concreto? Imagine 2 comentários em um único PR, ambos sobre early return, mas o primeiro comentário é pra simplificar 1 if-else enquanto o segundo é sobre 8 if-else-if aninhados, assim sendo basta dizer: “ei, meu primeiro comentário talvez não faça tanta diferença, mas eu insistiria que você ponderasse melhor a sugestão do segundo comentário”. Perceba que o mesmo tópico ao mesmo tempo teve tanto peso trivial quanto peso de maior importância para legibilidade, ninguém precisou decidir de forma definitiva se early return é bom ou não, ninguém ficou com medo de falar nada, cada caso é um caso, e a palavra final fica com quem é responsável pela tarefa. Pronto. É o famoso “depende”, mas mais otimizado.
Em times que participei, um hábito legal que aprendi foi de incluir em alguns comentários o emoji de 💅 indicando que aquilo é algo que talvez só eu dê importância visual. Ou ainda, para times gringos, marcar entre parênteses um “nit” de “nitpicking”, uma expressão popular usada para apontar uma crítica sem grande importância. Isso contextualiza bastante cada comentário, permitindo a troca de pontos de vista mas ainda evitando bloquear tarefas sem motivo.
Em resumo, subjetividade é importante para nos sentirmos bem mantendo um código e não é para ser ignorada, apenas controlada por pragmatismo.
3. Momento de QA no code review?
Bom… não se busca por bugs em PRs 🤐
Calma, calma. É óbvio que se você perceber algo estranho é só apontar. Mas este apenas não é (ou não deveria ser) o foco. Isso porque humano não compila código.
Não adianta se forçar a querer interpretar todo código na sua cabeça. Quer saber se algo produz a saída que você espera? Sugira um teste automatizado, a CI vai rodar e responder sua pergunta de forma mil vezes mais confiável e perene.
“Ah, mas tal corner case parece muito importante aqui”, ótimo, sugira um teste específico a respeito.
Às vezes é difícil escrever teste unitário ali por qualquer motivo (tipo simular race conditions). Ainda assim peça que a pessoa atribuída ao pull request pelo menos encontre meios de forçar a situação manualmente ou, em último caso, de explicar verbalmente medidas tomadas para descartar aquela preocupação. Relembre que um dos objetivos da revisão é apenas tomar ciência, então a conclusão pode ser apenas assumir um débito técnico consciente em prol de velocidade: “não sei se isso é um problema mesmo, então seguiremos em frente e observaremos se determinado sintoma surge em produção”, aí é vida que segue.
E para times sem testes automatizados ou sem integração contínua? Então de fato não tem o que fazer, sua gestão (literalmente) pagará o preço contratando mais pessoas testadoras para compensar. Neste cenário, pode ser uma boa ter pessoas de QA baixando o código do PR e testando manualmente — o que, na minha opinião, cria um gargalo um pouco chato.
Mas sempre tem muito bug entrando 💀
Se você já tem certeza de que todo PR tem bug, então evite ser forçado a enxugar gelo para sempre: tem problema maior de processo aí. Descubra e resolva. Talvez é a cultura do time promovendo a falta de autonomia e/ou responsabilidade das pessoas, ou falha de gestão técnica em promover boas práticas como testes, ambiente de homologação, bom DevOps etc. Este é um tópico que para ser expandido merece uma publicação inteira à parte, mas ficam meus dois centavos.
4. Sua tarefa não precisa ser 1 (um) pull request
Abra até 15 PRs se for necessário 😎
Supondo que seu time controla algum kanban de tarefas, lá vai ter um card/ticket/tarefa. E por alguma razão surgiu a alucinação coletiva (talvez pelo git flow?) de que cada PR deveria estar ligado unicamente a uma tarefa, 1 pra 1.
Não precisa. De verdade. Tu pode (ou deveria poder) quebrar seus PRs em vários.
Imagine que sua tarefa é “gerar relatório de vendas”, você passou o dia inteiro preparando a query no banco de dados que gera isso. Abra um PR ao fim só com isso, descreva ao time as dificuldades, talvez discutam armadilhas, testes, otimizações ou outra qualquer preocupação específica. Isso é progresso! E seu time vai revisar com atenção focada àquilo. No dia seguinte, você abre outro PR usando essa query no pra responder um endpoint HTTP ou algo assim. Seus dois PRs fazem parte da tarefa original “gerar relatório de vendas”, os stakeholders não deveriam ter interesse de saber se você precisou de 1, 2 ou até 15 PRs pra alcançar esse resultado, isso é detalhe de implementação.
“Mas minha tarefa vai estar no kanban como ‘em revisão’ desde o primeiro PR?” Sim, eu sei, é bem normal ter um kanban de desenvolvimento com essa coluna… mas honestamente tanto faz como você resolve isso. Desde que todas as pessoas estejam cientes. Entorte seus processos e ferramentas à forma como o time trabalha, não deixe que o contrário aconteça. Você pode, por exemplo, criar subtarefas, que podem ser desde uma simples checklist na descrição da tarefa ou algo mais enfeitado como o Jira que permite cada subtarefa ser um “subticket” com status diferente.
Separe refatoração de alteração 🙅
Um código refatorado terá seu código diferente porém resultado idêntico, o objetivo é melhorar a manutenibilidade. Já uma “alteração” implica em resultados diferentes, ou seja, ela resolve um bug, adiciona uma nova funcionalidade, muda interfaces públicas, coisas assim. Separar refatoração de alteração em PRs diferentes ajuda quem tá revisando a entender quais são seus objetivos ali. Se o objetivo de um PR é exclusivamente melhorar legibilidade, comentários de “perfumaria” talvez tenham um peso maior, por exemplo.
A branch que depende da branch que depende da… 🤪
Abrindo vários PRs, você pode se ver na situação de ter uma branch dependendo de outra que depende de outra e assim por diante. Se o time revisa com a frequência adequada, isso não acontece.
Isso é problema de branches envelhecidas, basta aumentar a frequência em que PRs são abertos e/ou ampliar a rotina de revisão.
O que não pode acontecer é manter uma árvore super complexa de branchs, tendo que fazer rebases e merges pra sincronizar tudo a cada minuto. Particularmente, no máximo eu chego a dois níveis de branchs privadas a partir de alguma pública, e mesmo assim é bem raro, só uma branch a partir da pública já devia servir.
5. Terminou o code review: dê a conclusão
Depois de escrever N comentários específicos em um PR, seja pragmático(a) e direto(a) sobre sua expectativa com um comentário geral.
Em termos práticos, usando as opções do GitHub, o que eu pessoalmente tenho de convenção é:
- ✅ ao aprovar, significa que mesmo que haja algum comentário específico, a pessoa responsável pode ignorar tranquilamente, pois não espero voltar aqui para re-revisar;
- 💬 ao apenas comentar é porque eu ficaria feliz de ver o que sugeri ser resolvido ou respondido, mas não ao ponto de insistir nem argumentar de volta, talvez eu nem volte para re-revisar; e
- ⛔️ ao solicitar alterações considero importantíssimo o que deixei dito, e desejo voltar para re-revisar para ver no mínimo um consenso ou boa resposta, isto aqui bloqueia as pessoas e uso com parcimônia.
No comentário geral, junto da “conclusão do GitHub”, também deixo claro se algum dos meus questionamentos em particular é mais digno de atenção que os outros, para deixar ainda mais transparente o que é preocupação real. Relembrando que o objetivo de revisar um PR é entender e ajudar o(a) autor(a).
Tudo isso é com o objetivo de não “atrasar” a vida de ninguém por nitpicking, mas ao mesmo tempo continuar estimulando uma cultura de troca de opiniões sem medo de pisar em ovos. 🙂
Uma dica final bem fácil ao revisar código é sempre pensar: se eu fosse tirar dinheiro da minha carteira para pagar a hora daquela pessoa lendo/aplicando a revisão, eu pagaria pelo que eu mesmo estou questionando/sugerindo? Essa alegoria é bem próxima da realidade, e inclusive uso ela para decidir se marco ou não reuniões!
Mazuh Softwares 