Code review é revisão de risco

Já li milhares de revisões de pull request e a maioria revisa a coisa errada. Nomes, formatação, se um helper deveria ser extraído, se os testes usam o estilo de asserção preferido. Tudo válido, tudo barato, tudo fora do ponto. O motivo de revisarmos código antes do merge é que o merge é uma decisão de aceitar um risco, e a revisão é o único momento em que esse risco ganha um segundo par de olhos. Se a revisão não fala de risco, é uma checagem de estilo com botão de merge.

As perguntas que importam

Quando reviso, seguro quatro perguntas na cabeça. O que essa mudança pode quebrar? Não em teoria, concretamente: quais tabelas, quais endpoints, quais jobs, quais clientes. Quem descobre primeiro se quebrar, e como: um alerta, um ticket de suporte, um relatório trimestral? Como desfazemos: um revert, uma feature flag, uma migração de dados para trás, ou é uma porta de mão única? E qual é o raio de explosão se o pior caso acontecer numa sexta à noite? Uma mudança que passa nas quatro é segura independentemente dos nomes. Uma que falha em uma merece conversa por mais limpa que pareça.

Revisar o diff não basta

O diff mostra o que mudou. O risco mora no que a mudança toca. Uma mudança de uma linha num utilitário compartilhado tem mais risco do que uma funcionalidade nova de duzentas linhas que ninguém chama ainda. Uma mudança num valor padrão afeta todo chamador que não o definiu explicitamente. Uma migração que adiciona uma coluna é segura; uma que renomeia quebra toda query que ainda usa o nome antigo, incluindo as do sistema de relatórios de que ninguém lembra. Então leio o diff, depois leio quem chama, depois quem chama quem chama, até acabar a surpresa.

Mudanças em dados têm outra régua

Um bug em código se conserta com um deploy. Um bug em dados se conserta com um engenheiro, na mão, de madrugada, com um backup aberto em outra janela. Qualquer mudança que escreva dados de forma diferente de antes, altere um esquema, faça backfill, apague ou transforme registros é revisada como se fosse cirurgia em produção, porque é. Quero ver a query que estima as linhas afetadas. Quero ver o rollback. Quero saber que foi rodada numa cópia antes. Uma revisão que aprova uma migração de dados em trinta segundos não é revisão.

Reversibilidade é a segurança mais barata

O comentário de revisão mais eficaz que deixo costuma ser alguma versão de: dá para tornar isso reversível? Coloque atrás de uma flag. Adicione a coluna nova antes de remover a antiga. Escreva o caminho novo ao lado do antigo e troque os leitores depois. Logue antes de impor. Cada uma dessas coisas transforma uma porta de mão única em uma de mão dupla, e portas de mão dupla podem ser atravessadas com muito menos escrutínio. A maior parte do risco de uma mudança não está na mudança em si, mas em quanto custa desfazê-la.

O que isso faz com um time

Times que revisam por risco escrevem código diferente. Passam a incluir o plano de rollback na descrição porque sabem que vai ser perguntado. Dividem mudanças assustadoras em passos seguros porque uma mudança pequena e reversível é aprovada em minutos e uma grande e irreversível ganha uma reunião. Param de discutir nomes, porque nome se conserta depois e tabela corrompida não. A revisão vira o lugar em que o time decide, junto, o que está disposto a perder. Era isso que ela deveria ter sido desde o início.