O principal objetivo da revisão de código é encontrar código difícil de manter

• A revisão de código está mais próxima de um processo para revelar com antecedência código que será difícil de manter no futuro do que de um procedimento para encontrar bugs ou garantir integridade
• Se até o revisor tem dificuldade para entender o código ao lê-lo, há grande chance de que o mesmo problema afete futuros mantenedores
• É melhor corrigir isso agora, enquanto o autor original ainda se lembra do contexto, e não é realista esperar que a inspeção de código encontre bugs de forma confiável
• Para o revisor, uma tarefa mais executável do que “encontre bugs” é “tente entender e marque as partes que não fizerem sentido”
• Uma boa revisão não começa tentando provar tudo com perfeição, mas sim deixando notas nos pontos que não forem compreendidos e pedindo melhorias

---

Mudando o foco da revisão de código

• O objetivo central da revisão de código não é que o revisor encontre bugs, nem garantir que o código não tenha bugs
• Na prática, é fraca a expectativa de conseguir encontrar bugs de forma geral apenas passando os olhos pelo código
• Por isso, o centro da revisão está menos em “o código está correto?” e mais em “outra pessoa conseguirá ler e corrigir isso depois?”

Código difícil de entender é sinal de risco de manutenção

• O revisor lê o código tentando entender o que ele faz e como funciona
• Partes que não são compreendidas se tornam um sinal de risco de que futuros mantenedores podem ficar bloqueados
• É melhor corrigir esse tipo de código imediatamente, enquanto o autor original ainda se lembra do contexto

Tarefas de revisão mais executáveis do que caçar bugs

• O pedido “tente encontrar bugs neste código” é uma tarefa cujo sucesso é difícil de avaliar
• Mesmo que alguns bugs sejam encontrados, outros podem continuar escondidos, o que para o revisor torna o fracasso mais fácil de ficar evidente
• Em contraste, o pedido “tente entender este código e aponte o que não conseguir entender” tem um critério mais claro
  • Não é necessário entender tudo perfeitamente
  • Basta registrar as partes que não foram compreendidas
  • Tentar entender o todo e deixar notas nos pontos em que travou já cumpre o papel da revisão

Critérios de revisão na prática

• Código que o revisor não consegue entender pode, por si só, se tornar alvo de correção
• Comentários de revisão não servem apenas para relatar bugs, mas também para revelar falta de explicação, problemas de estrutura e fluxos difíceis de ler
• Nesse critério, mais importante do que provar a validade do código é deixá-lo em um estado que outros membros da equipe consigam ler e trabalhar depois

Encontrar bugs é um efeito colateral

• Isso não significa que a revisão de código nunca encontre bugs
• Bugs podem ser descobertos durante a revisão, mas é difícil esperar que ela encontre todos os bugs ou a maioria deles
• Uma condição de sucesso mais realista é verificar a compreensibilidade e melhorar imediatamente, junto com o autor original, as partes que serão difíceis de manter

[GN⁺]

Opiniões do Hacker News

• Revisão de código não tem um único objetivo. Encontrar código difícil de manter é importante, mas pode não ser o único objetivo, nem talvez o mais importante
  Ela funciona como uma salvaguarda que dificulta o merge de código malicioso, mesmo que um desenvolvedor ou uma IA vá para uma direção estranha, e alguém que não esteja tão próximo do problema pode enxergar uma abordagem melhor ou problemas que passaram despercebidos
  Alguém que conhece melhor outras partes do sistema também pode detectar problemas de interação, faz com que pelo menos uma pessoa se familiarize com aquele código, e vira uma oportunidade de aprendizado tanto para o autor quanto para o revisor
  Isso é especialmente importante quando há diferença de senioridade: ao mentorar novos funcionários, adiciono-os como revisores em todos os meus PRs para que vejam como trabalho, e também reviso os PRs deles para ajudar a dar direção. Às vezes eu também aprendo com eles
  Encontrar bugs também faz parte, mas não deveria ser o mecanismo principal, e é especialmente importante para bugs de segurança e desempenho, que são difíceis de capturar com testes automatizados

  • Para acrescentar um argumento antigo: as pessoas escrevem código de forma diferente quando sabem que ele será revisado e que, com base nele, será formada uma impressão de longo prazo sobre a competência e adequação à organização de quem o submeteu
  • De modo parecido com o ponto 3, alguém mais familiarizado com aquele subdomínio pode apontar partes que não combinam com o código existente ou que podem causar problemas
  • Li “objetivo único” como significando: entenda o código e questione as partes que não são compreendidas. Se a ideia é que, depois de entender, você consiga apontar o que está errado, tolo ou inseguro, então faz sentido
    Isso vale especialmente para modularização e decomposição; depois de entender um PR enorme por completo, você forma um modelo mental e começa a enxergar se ele será sustentável, se um dia virará um pesadelo completo, ou algo intermediário

• Acho que talvez a parte mais importante da revisão de código seja a transferência de conhecimento
  Na nossa equipe pequena, salvo casos urgentes, a equipe inteira marca aprovação no PR antes do merge, e graças a isso todos os membros têm uma noção geral do estado atual da base de código. Isso evita surpresas como “o sistema inteiro do qual eu dependia desapareceu”, que já vivi em empresas mais isoladas em silos
  Também vira um espaço para fazer perguntas sobre como as coisas funcionam e aumentar o entendimento. Em uma equipe que funciona bem, todos os desenvolvedores deveriam entender em algum grau o sistema inteiro, incluindo partes em que não mexem diretamente
  Outra função importante é a checagem do conhecimento organizacional. Recentemente alterei um pouco uma tabela, e um colega avisou que um microsserviço que eu não tinha considerado escrevia nela e quebraria. Eu nem sabia que esse microsserviço existia, nem que acessava aquela tabela, mas graças a essa checagem conseguimos evitar um problema maior e uma situação de limpeza de dados

  • “A equipe pequena inteira marcar aprovação no PR antes do merge” parece bom em uma base de código pequena e de ritmo lento, mas, se for imposto a uma equipe grande ou a uma base de código rápida, tende a virar um ritual formal em que as pessoas passam os olhos no código e apertam o botão de aprovação
    No fim, todo mundo está ocupado com seu próprio trabalho e não quer ser a pessoa que bloqueia um PR importante, então apenas aprova, e na prática ninguém está revisando o código
  • Fico curioso sobre o tamanho da equipe. Imagino que esse modelo não escale quando passa de talvez cinco engenheiros
    Para problemas como “o sistema inteiro do qual eu dependia desapareceu”, acho que testes automatizados capazes de detectar isso mesmo sem a presença de quem depende daquele sistema são muito importantes
    Também sou forte defensor de propriedade compartilhada sobre tudo. É natural que as pessoas acabem praticamente donas de certas partes da base de código, especialmente componentes que criaram, mas isso leva a silos e a um baixo bus factor. Não deve existir uma estrutura em que uma pessoa é dona de um sistema, e esse sistema depende de outro componente pertencente a uma única pessoa
  • Fico curioso sobre o que teria impedido aquela mudança quebrada de ir para produção se o colega não estivesse na revisão. Se revisão de código é importante, onde entram os testes?
  • Às vezes crio um PR e marco outro desenvolvedor mesmo para código que vou fazer merge imediatamente de qualquer forma. O objetivo é oferecer um caminho fácil para ver o que foi mergeado e por quê, para que as pessoas não percam de vista o que há dentro da base de código
  • Para mudanças ou limpezas que não sejam triviais, é sempre bom ter um segundo par de olhos. Mas o modelo em que “todo mundo lê tudo” não escala para N grande
    Quando há coisa demais para ler, ninguém consegue acompanhar; por isso se delega, se cria documentação e se realizam sessões de visão geral

• Sempre achei melhor encarar a revisão de código como o portão pelo qual o código que era de propriedade do autor passa a ser propriedade da equipe ou do projeto
  O código que estou revisando não é o seu código; é um código que em breve será nosso. Naturalmente, manutenibilidade é um fator importante nisso

  • Que ambiente invejável e luxuoso
    Minha equipe começou a usar IA, então mudei para uma abordagem simples. Não deixo comentários; faço apenas um julgamento binário de aprovação: “isso está em um nível insano ou dá para passar?”
    Estou preservando meu tempo e minha saúde mental

• Fazer isso só torna o revisor e o autor mais preguiçosos
  O objetivo da revisão de código é multifacetado. O código é difícil de manter? Pode conter bugs? Dá para torná-lo mais simples e limpo? Está de acordo com o estilo de código do projeto? Faz outras pessoas entenderem o código? Ajuda a integrar membros juniores da equipe? Faz um sanity check das decisões de design? Tudo isso conta
  Textos leves assim, em geral, estão mais perto de uma justificativa que revisores de código preguiçosos dão a si mesmos

  • Há um checklist específico do que se deve observar em uma revisão
    É preciso verificar se, funcionalmente, ela atinge o objetivo descrito na issue ou no PR; se há código desnecessário, como saídas de debug restantes ou chaves de API privadas; e se há defeitos evidentes, como vazamentos de memória, casos de borda não tratados, falhas de segurança ou chamadas a APIs obsoletas
    Também é preciso ver se dá para tornar o código mais compreensível, se convém adicionar ou remover abstrações, se os nomes de variáveis e métodos são melhores, e se seria melhor usar mais ou menos estilo funcional
    Também é preciso confirmar se está consistente com a base de código ou o guia de estilo, se há melhorias óbvias de desempenho, como usar um conjunto hash em vez de uma lista ou aplicar avaliação preguiçosa, e se os testes são suficientes
    Também não concordo totalmente com a tese de que, se você não consegue entender o código, ele não deve passar. Alguns códigos são simplesmente muito difíceis de entender, e o objetivo é torná-los funcionalmente corretos e tão compreensíveis quanto possível
  • O setor tem se esforçado para passar de “culpar o autor” para “culpar o processo e a equipe”
    Mas este texto parece mais uma isca, parecido com dizer “as pessoas acham que jantar é comer comida, mas na verdade não é comer, é se conectar com família e amigos”. É um tipo específico de lógica reducionista frágil que funciona bem no HN
  • Agora que, por causa da IA, a velocidade de escrever e implantar código aumentou, a ênfase precisa se deslocar para a revisão. Precisamos verificar se o código realmente roda corretamente, se todas as nossas suposições estão certas e se não há efeitos colaterais não intencionais
    Senti que revisão e depuração consomem muito mais tempo do que escrever e produzir código, e simplesmente “rezar para funcionar” nunca termina bem

• A afirmação de que “em geral, não é possível encontrar bugs olhando o código” não está certa de forma alguma. É possível encontrar muitos em cada nível de abstração, e chamamos isso de code smell
  Descritores de arquivo não fechados, corrotinas sem await, blocos try/catch enormes que retornam algum valor sem registrar o erro, casts de tipo incorretos etc. entram nisso
  Como princípio geral, não se deve tratar o type checker, o compilador e o runtime como meras etapas pelas quais o código precisa passar. É preciso trabalhar com essas etapas, reconhecer que são ferramentas valiosas e não trabalhar contra elas

  • Não sei do que estão falando. Já encontrei bugs só com revisão de código, sem executar nada; outras pessoas também fizeram isso no meu código; e vi isso acontecer em revisões de outras pessoas
    Dá para definir “em geral” de algum modo que torne a frase tecnicamente verdadeira, mas aí ela perde quase todo o sentido

• Para fazer afirmações amplas sobre revisão de código, é importante definir de que tipo de revisão de código se está falando
  Hoje, a revisão assíncrona de PRs no estilo GitHub, com comentários inline, é o padrão, mas revisão não é só isso. Antigamente havia revisões presenciais com processos mais próximos de uma defesa de tese ou apresentação em conferência
  A literatura que diz que revisão de código é uma prática de qualidade útil — na verdade, uma das poucas práticas de qualidade úteis — vem, em geral, de processos de revisão muito mais estruturados do que os de hoje
  Pessoalmente, antes dos LLMs, eu achava que as revisões de PR no estilo GitHub serviam para fazer o processo parecer bom ou para preencher uma caixinha de governança; na era dos LLMs, acho que o custo-benefício ficou muito pior e elas devem desaparecer

  • Revisões síncronas ainda são possíveis. Um dos meus primeiros gerentes me ensinou que, se uma revisão de código “padrão” vai e volta mais de uma vez, quase sempre é melhor se encontrar pessoalmente — ou, se ao menos uma pessoa estiver remota, resolver por Zoom — e depois deixar o acordo registrado em um comentário
    Forçando uma analogia técnica, a comunicação assíncrona por texto tem mais perda e menor throughput do que a fala em termos da informação que consegue codificar com sucesso. Por isso, quando é preciso trocar muita informação, vale a pena pagar o custo de sincronização
  • Em um dos meus primeiros empregos, era preciso imprimir o pacote de mudanças, revisá-lo e assiná-lo. Havia até uma pessoa responsável por guardar a versão final em um arquivo físico
    Era mais próximo da engenharia tradicional, e todos tinham que pensar no software como algo mais permanente

• Garantir a manutenibilidade é, sim, um dos benefícios da revisão de código, mas dizer que esse é seu único objetivo é uma afirmação bastante ousada
  A revisão de código também é uma ferramenta para a equipe acompanhar mudanças no código e compartilhar a responsabilidade coletiva por toda a base de código

• Revisão de código não é uma coisa única. Há vários motivos, como compartilhamento de conhecimento, lavagem de responsabilidade, qualidade de código, conformidade regulatória etc.; e, como sempre, o objetivo depende do contexto de uso

  • Dizer que a maioria das pessoas não entende o motivo de uma revisão por pares soa bastante ignorante. Acreditar que existe apenas um único objetivo parece, por si só, sinal de pouca experiência trabalhando com outras equipes ou pessoas

• Se o autor é matemático, a frase “em geral, não é possível encontrar bugs olhando o código” provavelmente não quer dizer que encontrar bugs seja completamente impossível
  Deve estar mais perto de dizer que não é possível encontrar todos os bugs, ou encontrar um bug específico

  • Pela minha experiência em aulas de matemática na universidade, matemáticos muitas vezes são péssimos em se comunicar com outros seres humanos. Isso explica por que o que ele acha que disse e a forma como quase todo mundo lê a frase são diferentes
  • Se esse for o sentido, está certo
    Ligando isso ao ponto da manutenibilidade, um objetivo da revisão também é tornar o código o mais simples possível para aumentar a probabilidade de que ele seja depurável por revisão. Isso não impede bugs em sentido absoluto, mas aumenta a probabilidade
  • Parece que o autor, sendo matemático, não entendeu o sentido dos quantificadores em linguagem natural que usou. “Em geral, não é possível encontrar bugs olhando o código” significa “em geral, não é possível encontrar nenhum bug olhando o código”, não “não é possível encontrar todos os bugs”
    A primeira interpretação é relevante, mas falsa; a segunda é verdadeira, mas irrelevante
    Talvez o autor quisesse dizer “em geral, não é possível encontrar um determinado bug olhando o código”, ou seja, “para todo bug B, não é possível encontrar B”, mas isso também é verdadeiro e distante do ponto central

• Já trabalhei tanto com pessoas que frequentemente recusam sugestões em PRs quanto com pessoas que as aceitam
  Quem aceita sugestões me faz pensar que, em certa medida, está disposto a compartilhar comigo a propriedade do código. Dá a sensação de que ambos mantemos e entendemos o código, olhando na mesma direção
  Por outro lado, em PRs de pessoas que recusam sugestões, a vontade de participar diminui. Se vai ser recusado de qualquer forma, por que gastar tempo fazendo uma revisão cuidadosa?

  • Na nossa equipe, geralmente colocamos prefixos antes dos comentários, como thought, change, nit, fix, chat
    Por exemplo, thought seria algo como “talvez foo fique mais comum no futuro, então vamos refatorar quando isso acontecer”; change, “isso é uma abstração vazada, eu gostaria que fosse modelado como bar”; nit, “o nome não é muito intuitivo, então considere Baz ou Boo”; fix, “este teste unitário valida o campo errado”; chat, “esta é uma decisão grande que vai definir o formato das soluções dessa categoria daqui para frente, então vamos discutir primeiro com a equipe”
    Alguns prefixos significam que o PR fica bloqueado até a correção, enquanto outros indicam que o comentário pode ser aceito ou não. Para o autor, fica claro o que é algo em que “precisamos chegar ao mesmo entendimento” e o que é apenas uma “preferência de expressão” ou uma “observação”
    Porém, se você deixou um nit e a outra pessoa não concordou e ignorou, não deve se sentir ofendido. Se você se sentia fortemente sobre aquilo, então não deveria ter sido um nit
  • Se você acha que algo é importante, deve deixar uma sugestão bloqueante e forçar a conversa.