Diretrizes de Revisão de Código
O que os revisores procuram em pull requests de Python - correção, testes, segurança, operabilidade e convenções da equipe - sem transformar a revisão em críticas de estilo que o ruff já impõe.
Receita
Passagem de 5 minutos do revisor:
- Leia a descrição do PR - link do ticket, nota de migração, plano de rollout.
- Verifique a lista de arquivos - dependências inesperadas, lockfile, alembic, infra.
- Leia os testes primeiro - a mudança de comportamento está coberta?
- Verifique a implementação - casos extremos, erros, tipos.
- Comente com perguntas, não exigências - aprove quando a confiança + CI estiver verde.
Quando usar:
- Autor preparando PR para revisão
- Revisor abrindo a fila de notificações
- Tech lead calibrando a cultura de revisão
- Retrospectiva após defeito escapado
Exemplo de Trabalho
Boa descrição de PR:
## [BILL-91] Aplicar IVA em pedidos da UE
- Adiciona TaxService com matemática Decimal
- Revisão Alembic `a1b2c3` adiciona coluna `vat_amount` (nullable, preenchida em follow-up)
- Feature flag `billing.vat_enabled` padrão desativado
## Plano de teste
- [x] uv run pytest tests/billing/test_tax.py
- [x] Manual: criar pedido com endereço DE, flag ativada, ver linha de IVA
## Rollback
Desativar flag; migração reversível via downgrade (descarta coluna - OK pré-produção)Bom comentário de revisão:
**Pergunta:** O que acontece para itens de linha com quantidade zero - divisão por zero ou pular?
Sugiro assert em `test_tax_zero_quantity` - fico feliz em aprovar depois disso.O que isso demonstra:
- Descrição vincula ticket, migração, flag, rollback
- Plano de teste espelha comandos de CI
- Revisão faz pergunta + sugestão de teste concreta
- Decimal e feature flag chamados explicitamente
Mergulho Profundo
Como Funciona
- Responsabilidade do autor - Pequenos PRs, auto-revisão do diff, CI verde antes de solicitar revisão.
- Responsabilidade do revisor - Resposta em tempo hábil, priorizar risco, aprovar quando a barra for atingida.
- CODEOWNERS - Atribui automaticamente especialistas em domínios para caminhos sensíveis.
- CI como primeiro revisor - ruff, pytest, mypy, scan de segurança.
Lente de Revisão (ordem de prioridade)
| Prioridade | Verificação |
|---|---|
| P0 | Correção, segurança, risco de perda de dados |
| P1 | Testes, migrações, quebras de contrato de API |
| P2 | Desempenho, observabilidade (logs/métricas) |
| P3 | Estilo (se CI perdeu), nomenclatura, documentação |
Foco Específico de Python
- Async - Chamadas bloqueantes em
async def? - Dinheiro - float usado para moeda?
- SQL - Apenas consultas parametrizadas?
- Pickle/yaml load - Desserialização insegura?
- Dependências - Novos pacotes transitivos justificados no diff do lockfile?
- Tipagem - API pública anotada?
Armadilhas
- Aprovação cega de CI verde - Testes podem não cobrir o branch. Correção: ler as asserções dos testes, não apenas a cobertura %.
- Comentários de estilo que o ruff impõe - Desperdiça tempo do autor. Correção: comentar apenas se a regra deve mudar no pyproject.
- Bloqueio por preferência - "Eu usaria dict em vez de dataclass". Correção: pedir ADR se for arquitetural.
- PRs enormes - 50 arquivos não revisáveis. Correção: autor divide; revisor solicita divisão antes de leitura profunda.
- Revisão de migração ausente - Deploy falha em revisão duplicada. Correção: verificar a cadeia alembic e a nota de downgrade.
- Tom indelicado - Aumenta o bus factor. Correção: formulação colaborativa; elogiar bons padrões.
Alternativas
| Alternativa | Usar Quando | Não Usar Quando |
|---|---|---|
| Programação em par | Mudança complexa e arriscada | Dispersão de fuso horário para revisão assíncrona |
| Bot de revisão de IA | Primeira passagem de críticas | Único revisor para segurança |
| Documento de design antes do código | Grande mudança arquitetural | Correção de bug de uma linha |
| Sem revisão (individual) | Protótipo descartável | main de produção |
FAQs
Quão rápido a revisão deve acontecer?
Resposta inicial alvo < 1 dia útil; SLA por wiki da equipe.
Aprovar com nits?
Sim, se os nits forem não bloqueantes e rastreados em um ticket de acompanhamento opcional.
Solicitar alterações vs comentar?
Solicitar alterações quando a mesclagem causaria risco de produção; comentar para fins educacionais ou opcionais.
Júnior revisa sênior?
Encorajado - perguntas melhoram a documentação; sênior ainda é CODEOWNER em caminhos críticos.
Auto-merge no verde?
OK para atualizações de patch do dependabot com política; não para migrações sem intervenção humana.
Revisar notebooks?
Preferir .py pareados do Jupytext ou revisar apenas o diff com saídas removidas.
Gatilho de revisão de segurança?
Autenticação, criptografia, novos parsers de entrada externa, atualizações de versão principal de dependência - marcar o contato de segurança.
Gatilho de revisão de desempenho?
Consultas de caminho crítico, novos loops ORM N+1, grandes quantidades de pandas no caminho de requisição.
Quem mescla?
Autor após aprovação + CI verde, a menos que o gerente de lançamento seja para o trem.
Monitoramento pós-merge?
Autor observa os dashboards por 30 minutos após a implantação de sua alteração - anotar no template do PR.
Relacionado
- Convenções e Guia de Estilo - idiomatismos
- Melhores Práticas de Git - higiene de PR
- Cultura de Revisão de Código - liderança
- 30 Regras de Segurança - lente de segurança
Versões de Stack: Esta página foi escrita para Python 3.14.0 (estável 3.14, manutenção 3.13), FastAPI 0.115+, Django 5.2, Flask 3.1, Pydantic 2, PyTorch 2.6+, pandas 2.2+, Polars 1.x, ruff 0.9+, e uv 0.6+.