Revisao De Pr

Revisão de pull request

Ordem da revisão

Revise nesta ordem e pare no primeiro nível que reprovar. Apontar nomes de variável num PR que tem race condition desperdiça o tempo de todo mundo.

1. Faz o que promete? Compare o diff com a descrição do PR. Código a mais que ninguém pediu é tão problema quanto código a menos.
2. Está correto? Casos de borda, nulos, listas vazias, erro de rede, concorrência.
3. Quebra alguém? Contrato de API, schema, formato de dados persistidos, comportamento que outro time consome.
4. Dá pra manter? Duplicação, acoplamento, nomes.
5. Estilo. Só se o linter não pega. Se pega, o comentário é no linter, não no PR.

O que buscar ativamente

Estas são as classes de defeito que passam por revisão humana com mais frequência:

• Erro engolido — catch que loga e segue, ?. mascarando estado inválido.
• Concorrência — leitura e escrita sem transação, await dentro de laço mutando estado compartilhado.
• Fronteira de confiança — entrada de usuário chegando em query, path ou shell sem validação.
• Vazamento — credencial, token ou PII em log, mensagem de erro ou resposta.
• Migração destrutiva — DROP/ALTER sem plano de rollback, ou incompatível com a versão anterior rodando em paralelo durante o deploy.
• Teste que não testa — asserção sobre mock, expect(true), teste que passa com a implementação removida.

Ao revisar código gerado por agente

Peso extra em: dependência que não existia no projeto, tratamento de erro excessivamente defensivo, testes que espelham a implementação em vez do requisito, e código morto deixado para trás. Verifique também se APIs citadas existem de fato na versão em uso.

Como escrever o comentário

Diga o que quebra e em qual cenário. Sem cenário concreto, é preferência.

# ruim
Isso aqui não parece thread-safe.

# bom
Duas requisições simultâneas para o mesmo `orderId` leem o estoque antes de qualquer
escrita, e as duas passam na checagem — dá pra vender mais do que existe. Precisa de
lock na linha ou de uma constraint no banco.

Separe bloqueio de sugestão. Prefixe o que não bloqueia com nit: e deixe explícito que pode ser ignorado.

Limiares padrão

Pontos de partida com base defensável, não leis. Onde o time já tem número próprio, o dele vale — mas conheça o motivo antes de afrouxar.

• PR acima de ~400 linhas alteradas: peça para quebrar. A quantidade de defeito encontrado por revisão despenca conforme o diff cresce — não porque o código fica melhor, mas porque a atenção do revisor acaba. Tire da conta arquivo gerado, lockfile e arquivo só movido de lugar.
• PR parado há mais de um dia: revise ou passe adiante explicitamente. Branch envelhecendo acumula conflito, e o custo disso supera o do review que ficou para depois.
• Duas aprovações quando o diff toca autenticação, autorização, migração de dados, cobrança, ou qualquer coisa que rode com privilégio elevado. Uma aprovação no resto.
• Cobertura: exija teste para o comportamento novo, não um percentual. Meta global premia teste de getter e não diz nada sobre o caminho que quebra em produção.
• Dono do código: se o repositório tem .github/CODEOWNERS, o caminho tocado determina quem precisa aprovar, independente do resto.

Antes de aprovar

• O CI passou — não aprove verde-pendente
• Migrações têm rollback e são compatíveis com a versão anterior
• Nada sensível em log ou mensagem de erro
• Os testes falham se a implementação for revertida
• A descrição do PR explica por que, não só o que