Code Review

Code review é uma das práticas mais importantes para manter qualidade de código e compartilhar conhecimento.

Objetivos do Code Review

✅ Qualidade: Encontrar bugs e problemas antes de production
✅ Conhecimento: Compartilhar contexto e aprendizado
✅ Consistência: Manter padrões de código
✅ Mentoria: Oportunidade de aprendizado mútuo
✅ Documentação: Histórico de decisões nos comentários

Para Autores (Quem Abre o PR)

Antes de Abrir o PR

Checklist pré-PR:

Código funciona localmente
Todos os testes passam (pytest para backend, npm test para frontend)
Linter passou (ruff check . para Python, npm run lint para JS)
Formatter aplicado (black . para Python, prettier para JS)
Commits organizados com mensagens claras (Conventional Commits)
Branch rebased com dev
Self-review feito (li meu próprio código criticamente)
Documentação atualizada se necessário
Screenshots adicionados se mudança visual

Self-Review

Antes de pedir review, revise seu próprio código:

# Ver diff completo
git diff origin/dev...HEAD

# Revisar commit por commit
git log origin/dev..HEAD --oneline
git show <commit-hash>

Perguntas para si mesmo:

Esse código faz sentido para alguém que não tem contexto?
As variáveis têm nomes claros?
As funções são pequenas e focadas?
Os testes cobrem os casos principais?
Há algum código duplicado?
A solução é a mais simples possível?
Adicionei TODO sem issue/RFC associada?

Escrever Boa Descrição de PR

Template de PR:

## Descrição

[Explique o que este PR faz e por quê]

Closes #123

## Tipo de Mudança

- [ ] Bug fix
- [ ] Nova feature
- [ ] Breaking change
- [ ] Documentação
- [ ] Refatoração

## Como Testar

1. Fazer checkout da branch
2. Rodar `pip install -r requirements.txt`
3. Executar `pytest tests/test_feature.py`
4. Acessar http://localhost:8000/api/endpoint
5. Verificar que...

## Screenshots (se aplicável)

[Adicionar screenshots ou GIFs]

## Checklist

- [ ] Código segue padrões do projeto
- [ ] Self-review feito
- [ ] Comentários adicionados em código complexo
- [ ] Documentação atualizada
- [ ] Testes adicionados/atualizados
- [ ] Todos os testes passam
- [ ] Sem avisos do linter
- [ ] Changes são backward compatible (ou documentadas)

Tamanho do PR

Ideal: 200-400 linhas de código

Pequeno (< 200 linhas): - Fácil de revisar - Feedback rápido - Menos bugs - Merge mais rápido

Grande (> 400 linhas): - Difícil de revisar bem - Feedback demora - Mais bugs passam - Considere quebrar em múltiplos PRs

Exceções OK: - Migrations geradas - Código gerado (OpenAPI, protobuf, etc.) - Refatoração grande mas mecânica - Documentação extensa

Durante o Review

Responder Comentários:

✅ Bom: - "Boa ideia! Vou mudar" - "Não tinha pensado nisso, faz sentido" - "Mudei conforme sugestão, veja commit ABC" - "Pode elaborar? Não entendi a preocupação"

❌ Evitar: - "Funciona do jeito que está" - Ignorar comentários - Ser defensivo - "Isso é estilo pessoal"

Pedir Esclarecimentos:

Não entendi por que isso seria um problema. 
Pode dar um exemplo de quando falharia?

Discordar Construtivamente:

Entendo a preocupação, mas escolhi esta abordagem porque [razão].
A alternativa teria [trade-off]. O que acha?

Agradecer Feedback:

Sempre agradeça review, mesmo se for crítico. Reviewer está ajudando!

Fazer Mudanças

# Fazer mudanças solicitadas
# Edit files...

# Commit
git add .
git commit -m "refactor: extract validation to separate function

Per review feedback"

# Push
git push origin feature/my-branch

Não fazer force push sem necessidade - Preserva histórico de discussão.

Para Revisores

Quando Revisar

Tempo de resposta: até 24h (melhor esforço)
PRs urgentes: avisar autor se não puder revisar logo
Não pode revisar: desfaça-se do PR e mencione outra pessoa

Checklist de Review

Funcionalidade

Entendo o que o código faz?
Resolve o problema descrito na issue?
Não introduz bugs óbvios?
Edge cases estão cobertos?
Erros são tratados adequadamente?

Código

Código é legível e autoexplicativo?
Nomes de variáveis/funções são claros?
Funções são pequenas e focadas (< 50 linhas)?
Não há duplicação desnecessária?
Lógica complexa está comentada?
Segue padrões do projeto?

Testes

Testes foram adicionados para novo código?
Testes testam comportamento, não implementação?
Testes são determinísticos (não flaky)?
Coverage não diminuiu?
Testes têm nomes descritivos?

Arquitetura

Está alinhado com arquitetura existente?
Não cria acoplamento desnecessário?
Abstrações fazem sentido?
Não há overengineering?

Segurança

Performance

Sem N+1 queries óbvias?
Não há loops desnecessários?
Queries de banco otimizadas?
Sem memory leaks potenciais?

Documentação

Docstrings atualizadas?
README atualizado se necessário?
API docs atualizadas?
Comentários explicam "porquê", não "o quê"?

Tipos de Comentários

Use prefixos para clareza:

[nit] - Nitpick, sugestão menor, não bloqueante

[nit] Poderia usar f-string aqui para melhor legibilidade

[question] - Pergunta para entender melhor

[question] Por que usamos timeout de 30s aqui? Há um caso específico?

[suggestion] - Sugestão de melhoria, considerar mas não obrigatório

[suggestion] Que tal extrair isso em um helper? Pode ser útil em outros lugares

[blocker] - Precisa ser resolvido antes do merge

[blocker] Este código não trata erro de conexão. Pode causar crash em prod

[praise] - Reconhecer código bom!

[praise] Adorei essa abstração! Muito mais limpo que antes

Escrever Feedback Construtivo

✅ Feedback Bom

Esta função está fazendo muitas coisas. Que tal extrair a validação 
para uma função separada? Ficaria mais fácil de testar e reutilizar.

Exemplo:
def validate_input(data):
    if not data.get('email'):
        raise ValidationError('Email required')
    # ...

Por quê funciona: - Explica o problema - Sugere solução específica - Dá exemplo de código - Tom colaborativo

❌ Feedback Ruim

Essa função está ruim. Refatore isso.

Por quê não funciona: - Vago e não específico - Não explica o problema - Não sugere solução - Tom negativo

Frameworks Úteis

AID (Action, Impact, Desired):

[Action] Esta função tem 150 linhas
[Impact] Difícil de entender e testar
[Desired] Que tal quebrar em funções menores focadas em uma responsabilidade?

SBI (Situation, Behavior, Impact):

[Situation] Na função process_payment()
[Behavior] Estamos fazendo 3 chamadas ao banco sem transaction
[Impact] Se falhar no meio, fica em estado inconsistente. 
Podemos usar with transaction.atomic() aqui?

Aprovar ou Rejeitar

Aprovar ✅

Aprovar quando: - Código atende requisitos - Sem problemas blockers - Nits e sugestões são menores

Comentário de aprovação:

LGTM! 🚀

Algumas sugestões menores mas nada blocker. 
Ótimo trabalho com os testes!

Request Changes 🔄

Request changes quando: - Há issues blockers - Código não funciona - Problemas de segurança - Falta testes críticos

Comentário:

Quase lá! Alguns pontos que precisam de atenção:

1. [Blocker] Função X não trata erro Y
2. [Blocker] Faltam testes para caso Z

Deixei comentários inline. Avise quando atualizar!

Comment (sem aprovar/rejeitar)

Usar quando: - Tem dúvidas mas não quer bloquear - Está revisando apenas parte do código - Quer dar feedback mas não é sua área

Boas Práticas de Review

✅ Fazer: - Assumir boa intenção - Fazer perguntas ao invés de acusar - Dar exemplos concretos - Reconhecer código bom - Ser específico - Explicar o "porquê" - Oferecer ajuda

❌ Evitar: - Ser condescendente - "Eu nunca faria assim" - Bike-shedding (focar em detalhes irrelevantes) - Reescrever código do autor - Aprovar sem ler - Comentários passivo-agressivos

Review de Código não é...

❌ Oportunidade para mostrar superioridade
❌ Lugar para impor preferências pessoais
❌ Discussão sobre tabs vs spaces
❌ Reescrever código do zero

Review de Código é...

✅ Melhorar qualidade do código
✅ Compartilhar conhecimento
✅ Encontrar bugs
✅ Manter padrões
✅ Oportunidade de mentoria

Situações Especiais

PRs Grandes

Se PR é muito grande:

Pedir para quebrar em PRs menores (se possível)
Revisar em múltiplas sessões
Focar primeiro em arquitetura/lógica
Depois revisar detalhes

Discordância

Se discorda fortemente:

Comentar no PR explicando preocupação
Se não resolver, escalar para Tech Lead
Se ainda não resolver, RFC pode ser necessária
Não bloquear indefinidamente - decisão precisa ser tomada

PRs de Pessoas Sênior

Mesmo código de pessoas sênior deve ser revisado:

Todos cometem erros
Oportunidade de aprendizado para todos
Compartilha contexto com o time
Mantém padrões

Métricas

Tempo de Review: - Objetivo: < 24h para primeiro feedback - < 48h para aprovar/rejeitar

Tamanho de PR: - Objetivo: 200-400 linhas - Alerta se > 600 linhas

Ciclos de Review: - Objetivo: Máximo 3 ciclos - Se mais, considere pair programming ou reunião

Exemplos

Exemplo de Review Excelente

## Geral

Ótimo trabalho! A lógica de validação ficou muito mais clara. 
Alguns pontos para consideração:

## app/services/payment.py

**L45-60 [suggestion]:**
```python
def process_payment(amount, user_id):
    # Função está fazendo muitas coisas
    ...

Que tal extrair a lógica de validação?

def validate_payment(amount, user_id):
    if amount <= 0:
        raise ValueError("Invalid amount")
    # ...

def process_payment(amount, user_id):
    validate_payment(amount, user_id)
    # resto da lógica

Benefícios: - Mais fácil de testar validação isoladamente - Pode reutilizar em outros lugares - Mais legível

L78 [question]: Por que timeout é 30s aqui? Há algum requisito específico? Se não, poderia usar constante do config?

L92 [blocker]:

result = api.call()  # Pode lançar exception
return result.data

Sem tratamento de erro. Se API falhar, vai crashar. Sugestão:

try:
    result = api.call()
    return result.data
except APIError as e:
    logger.error(f"API call failed: {e}")
    raise PaymentError("Failed to process") from e

tests/test_payment.py

L23 [praise]: Adorei este teste! Mock está bem feito e caso de teste é claro.

L45 [nit]: Nome do teste poderia ser mais descritivo: test_payment → test_payment_fails_with_invalid_amount

Resumo

1 blocker (tratamento de erro)
Algumas sugestões para considerar
Testes estão ótimos!

Avise quando atualizar. Bom trabalho! 👍 ```

Ferramentas

GitHub Features

Code suggestions: Sugerir mudanças que autor pode aplicar com 1 click
Line comments: Comentar linhas específicas
Review comments: Comentar no PR geral
Emojis: Usar emojis ajuda tom de comunicação 👍 ✅ 🎉 🤔

Automação

CI checks: Linter, testes, coverage
Danger: Bot que adiciona comentários automáticos
CodeClimate: Análise de qualidade de código