Code Review Checklist: What I Look for After 10 Years of PRs \u2014 COD-AI.com

Três anos atrás, aprovei um pull request que custou à minha empresa $47.000 em receita perdida durante um único fim de semana. O código parecia bom. Os testes passaram. A lógica parecia sólida. Mas eu perdi algo sutil na forma como lidávamos com as conversões de fuso horário para nosso sistema de cobrança por assinatura, e clientes em certas regiões foram cobrados duas vezes.

Esse incidente mudou a forma como eu reviso código para sempre. Eu sou Sarah Chen, e tenho sido gerente de engenharia sênior em três diferentes empresas de SaaS na última década, revisando uma média de 15-20 pull requests por semana. Isso dá aproximadamente 8.000 PRs na minha carreira. Eu já vi códigos brilhantes que entregaram bugs, e códigos desorganizados que funcionaram perfeitamente em produção por anos. Aprendi que revisar código não é sobre encontrar erros de sintaxe—seu linter faz isso. É sobre pegar os problemas invisíveis que apenas a experiência pode revelar.

Este artigo é a lista de verificação que eu gostaria de ter quando comecei. Não é abrangente—nenhuma lista de verificação poderia ser—mas representa os padrões que aprendi a reconhecer, as perguntas que treinei para fazer e os modelos mentais que salvaram minhas equipes de inúmeros incidentes de produção.

Os Primeiros 30 Segundos: O Que a Descrição do PR Te Diz

Antes de olhar para uma única linha de código, eu passo 30 segundos lendo a descrição do PR. Isso pode parecer inverso, mas descobri que 60% dos PRs problemáticos se revelam imediatamente através de comunicação deficiente. Uma boa descrição de PR responde a três perguntas: o que mudou, por que mudou e o que pode dar errado.

Quando vejo uma descrição que diz "bug corrigido" ou "componente atualizado", eu imediatamente sei que essa revisão vai levar mais tempo. O autor não pensou nas implicações de suas mudanças, o que significa que eu preciso fazer esse trabalho por eles. Por outro lado, quando vejo uma descrição que diz "Refatorado a autenticação de usuário para usar tokens JWT em vez de cookies de sessão. Isso reduz a carga no banco de dados em 40% durante horários de pico, mas requer que os clientes gerenciem a atualização do token. Risco: versões existentes do aplicativo móvel (< 2.3) precisarão de nova autenticação," eu sei que o autor fez sua lição de casa.

Eu procuro métricas específicas na descrição. Não melhorias vagas, mas números reais. "Desempenho melhorado" não significa nada. "Reduzido o tempo de resposta da API de 340ms para 180ms para o endpoint do perfil do usuário sob 1000 requisições simultâneas" me diz que o autor mediu seu trabalho e entende o impacto. Na minha experiência, desenvolvedores que incluem métricas nas descrições de seus PRs escrevem códigos mais reflexivos no geral.

A descrição também deve linkar para contextos relevantes—o ticket, o documento de design, o thread do Slack onde a abordagem foi discutida. Se eu estou revisando um PR isoladamente, sem entender o contexto mais amplo, vou perder coisas. Uma vez, aprovei uma "refatoração simples" que quebrou sem querer uma integração crítica porque eu não sabia sobre uma dependência que não estava documentada em nenhum lugar exceto em um thread de e-mail de três meses atrás.

Bandeiras vermelhas nas descrições de PR incluem: nenhuma descrição, descrições que são mais longas que as mudanças de código em si (geralmente indica sobreengenharia), descrições que dizem "WIP" ou "rascunho", mas o PR está marcado como pronto para revisão, e descrições que incluem frases como "não tenho certeza se esta é a abordagem certa" (então por que você está me pedindo para revisá-la?).

Tamanho Importa: A Regra das 400 Linhas

Eu tenho uma regra rígida: eu não reviso minuciosamente PRs que têm mais de 400 linhas de mudanças de código efetivas (excluindo código gerado, arquivos package-lock e fixtures de teste). Isso não é arbitrário. Pesquisas da Cisco e da SmartBear mostram que a eficácia da revisão de código cai dramaticamente após 400 linhas, e minha própria experiência confirma isso. Depois de revisar cerca de 200 linhas de código, meu cérebro começa a não prestar atenção nos detalhes. Com 400 linhas, basicamente estou apenas folheando.

"A revisão de código não é sobre encontrar erros de sintaxe—seu linter faz isso. É sobre pegar os problemas invisíveis que apenas a experiência pode revelar."

Quando alguém envia um PR de 1.200 linhas, eu peço que quebrem. Não me importa se tudo é "relacionado". PRs grandes são onde os bugs se escondem. Eu já vi vulnerabilidades críticas de segurança passar despercebidas em PRs de refatoração massiva porque os revisores se cansaram e pararam de prestar atenção por volta da linha 600. A vulnerabilidade estava na linha 847.

Claro, há exceções. Às vezes você precisa mover arquivos, ou atualizar código gerado, ou fazer mudanças abrangentes para atender a um novo padrão de lint. Nesses casos, eu peço ao autor para separar as mudanças mecânicas das mudanças lógicas. Envie os movimentos de arquivos em um PR, as mudanças de lógica real em outro. Isso torna ambos os PRs mais fáceis de revisar e mais fáceis de reverter se algo der errado.

Eu também presto atenção na proporção de código adicionado versus código deletado. Na minha experiência, os melhores PRs têm uma contagem líquida de linhas negativa—eles alcançam algo enquanto tornam a base de código menor. Quando vejo um PR que adiciona 500 linhas e deleta 50, começo a me suspeitar. Estamos adicionando complexidade? Estamos duplicando funcionalidades existentes? O autor está ciente do que já está na base de código?

A questão do tamanho se estende a arquivos individuais também. Se um PR toca 30 arquivos diferentes, mesmo que a contagem total de linhas seja razoável, isso é uma bandeira vermelha. Sugere que a mudança é mal limitada ou que a base de código tem problemas de acoplamento. De qualquer forma, merece uma análise mais cuidadosa.

A Camada de Lógica de Negócios: Onde a Maioria dos Bugs Se Esconde

Depois de uma década de revisões de código, posso te dizer onde os bugs se escondem: na camada de lógica de negócios, especificamente nos casos extremos e nas transições de estado. Não nos componentes da UI. Não nas consultas de banco de dados. No código que implementa suas reais regras de negócios.

Quando reviso a lógica de negócios, estou procurando por suposições. Cada suposição é um potencial bug. Uma vez revisei um código para um sistema de cálculo de desconto que assumiu que todos os descontos eram percentuais. O código funcionou perfeitamente até que o marketing quisesse oferecer uma promoção de "$10 de desconto". O sistema quebrou porque alguém dividiu pelo valor do desconto, e dividir por 10 quando você esperava um número entre 0 e 1 produz resultados muito errados.

Eu me pergunto: o que acontece nas fronteiras? E se a entrada for zero? E se for negativo? E se for nula? E se for uma string vazia versus uma string nula? E se o array estiver vazio? E se o usuário não tiver permissões? E se eles tiverem todas as permissões? E se o banco de dados não retornar resultados? E se retornar um milhão de resultados?

Procuro números mágicos na lógica de negócios. Quando vejo if (user.loginAttempts > 5), pergunto: por que 5? Isso está documentado? É configurável? O que acontece se quisermos mudar para 3 para certos tipos de usuários? Números mágicos na lógica de negócios são dívidas técnicas esperando para acontecer.

Máquinas de estado são outra fonte comum de bugs. Quando vejo código que gerencia transições de estado—status do pedido, ciclo de vida do usuário, estágios de fluxo de trabalho—eu desenho o diagrama de estado na minha cabeça. Todas as transições estão contabilizadas? Você pode entrar em um estado inválido? Uma vez peguei um bug onde um usuário poderia ser simultaneamente "ativo" e "suspenso" porque o código que definia um status não verificou o outro.

Eu também observo a lógica de negócios que está espalhada por várias camadas. Se eu vejo validação na UI, mais validação na API, e ainda mais validação na camada do banco de dados, eu sei que teremos inconsistências. As regras de negócios devem viver em um só lugar, e esse lugar deve ser óbvio...