Code review é a prática mais barata de formação técnica que existe e a que mais gera ressentimento em time.
As duas coisas dependem da mesma variável: como o comentário é escrito.
O comentário que não ensina nada
Isso está errado.Por que você não usou o padrão que já existe no projeto?Refaz.Os três compartilham um problema: quem lê descobre que errou e não descobre nada além disso. Se souber consertar, conserta. Se não souber, vai adivinhar, e a próxima versão vem com outro erro.
O segundo é o pior dos três, porque a pergunta não é pergunta. É uma acusação com ponto de interrogação, e todo mundo percebe.
A estrutura que funciona
Três partes: o que, por quê, e o caminho.
Essa query roda dentro do laço, então vira uma consulta por item —
50 pedidos viram 51 idas ao banco.
Dá pra buscar todos de uma vez antes do laço:
const customers = await repo.findBy({ id: In(ids) });
const byId = new Map(customers.map(c => [c.id, c]));
Escrevi sobre o padrão aqui: <link>A pessoa entende o problema, entende por que é problema e sai sabendo resolver o próximo. O custo pra quem revisa é um minuto a mais. O retorno é não precisar comentar de novo.
Separe o que bloqueia do que é gosto
O maior gerador de atrito em revisão é a ambiguidade sobre o que precisa mudar.
Uso prefixo explícito:
bloqueia: essa rota não confere se o pedido é do usuário logado.
sugestão: dava pra extrair essa parte pra uma função, se você achar que fica melhor.
dúvida: esse timeout de 30s foi escolhido por algum motivo específico?
elogio: gostei de como você isolou o cálculo. ficou fácil de testar.Sem isso, todo comentário parece obrigatório, e a pessoa refaz coisa que não precisava. Com isso, o autor sabe onde tem escolha.
O elogio não é enfeite. Revisão que só aponta erro treina o time a associar PR com julgamento, e o efeito prático é PR gigante que ninguém consegue revisar, porque abrir cedo virou exposição.
O que realmente merece bloqueio
Nem tudo é igual. A minha ordem:
Correção. Está errado, quebra caso conhecido, tem corrida.
Segurança. Falta autorização, injeção possível, segredo exposto.
Perda de dado. Migração sem volta, delete sem escopo.
Manutenibilidade grave. Ninguém vai entender isso em seis meses, incluindo quem escreveu.
Abaixo disso, é conversa. Nome de variável, ordem de função, preferência de estilo: se importa a ponto de bloquear, vira regra de lint. Discutir formatação em revisão é desperdício de duas pessoas, e o Prettier resolve sem mágoa.
Pergunte antes de afirmar
Boa parte do que parece erro em revisão tem um motivo que você não conhece.
Reparei que aqui não tem transação. Foi de propósito?
Fiquei em dúvida se o outbox cobre esse caso.Duas coisas acontecem. Se tinha motivo, você aprendeu algo. Se não tinha, a pessoa percebe sozinha, o que ensina bem mais que ser corrigida.
A pergunta genuína também protege quem revisa de estar errado em público, o que acontece com mais frequência do que a maioria admite.
O tamanho do PR é responsabilidade dos dois
Revisão boa em PR de 2.000 linhas não existe. Depois de uns 400 linhas, a capacidade de encontrar defeito cai drasticamente, e o que sai é aprovação com dois comentários cosméticos.
Se PR grande é frequente no time, o problema não é a revisão, é o fluxo. Vale conversar sobre fatiar, sobre flag de funcionalidade, sobre entregar em partes. E vale quem revisa dizer “não consigo revisar isso direito” em vez de aprovar.
O tempo de resposta importa mais que a profundidade
PR parado dois dias custa mais que revisão superficial. A pessoa perdeu o contexto, o branch divergiu, e o trabalho seguinte está bloqueado.
Um combinado que funciona bem: primeira resposta em até quatro horas úteis, nem que seja “peguei, olho hoje à tarde”. O que trava time não é a revisão demorada, é a incerteza sobre quando ela vem.
Do outro lado
Se você é quem recebe: descreva o PR. O que muda, por que, o que você considerou e descartou, onde você tem dúvida.
Três linhas de contexto economizam cinco comentários. E marcar explicitamente “aqui eu não tive certeza” direciona a atenção pra onde ela vale mais.
E quando o comentário parecer injusto, responda com a razão técnica, não com a defesa. “Fiz assim porque X” abre conversa. “Sempre fizemos assim” encerra.
Por que isso é carreira
Revisar bem é a forma mais rápida de virar referência num time. Você lê mais código que qualquer pessoa, forma um mapa do sistema que ninguém mais tem, e vira quem é consultado antes das decisões.
E é uma das poucas habilidades técnicas que continuam valendo quando o cargo muda de nome. Escrevi sobre o resto desse conjunto em de júnior a pleno.