Trabalho Por Equipe DevBras

Capa do artigo: Pull request pequeno é revisado de verdade: como quebrar o grande em partes

Pull request pequeno é revisado de verdade: como quebrar o grande em partes

Separe formatação, refatoração e mudança de comportamento em pull requests próprios, empilhe quando um depende do outro e descreva o que testar para a revisão achar o bug antes da produção.

O pull request "Novo cálculo de frete" chegou com 38 arquivos, 1.240 linhas adicionadas e 610 removidas. Trinta desses arquivos eram só formatação, porque o editor rodou o Prettier na pasta inteira. O revisor rolou a tela por vinte minutos, comentou duas vírgulas e aprovou. O bug, que cobrava frete em dobro para CEPs de outra região, estava em doze linhas do arquivo frete.js. A resposta curta: separe cada tipo de mudança no seu próprio pull request. Formatação sozinha, refatoração sem mudança de comportamento sozinha, e a mudança de comportamento por último, pequena o bastante para ser lida inteira, com uma descrição que diz o que testar.

Revisão não é carimbo. Ela só encontra bug quando o revisor consegue manter a mudança inteira na cabeça.

Por que o pull request grande é aprovado sem ser lido

A atenção de quem revisa acaba. Estudos sobre revisão de código costumam citar algo entre 200 e 400 linhas como a faixa a partir da qual a taxa de defeitos encontrados cai bastante. Acima disso, o revisor passa a procurar padrões superficiais, como nomes e estilo, porque não consegue mais acompanhar a lógica.

Misturar tipos de mudança piora o problema. Quando formatação e lógica estão no mesmo diff, o revisor não sabe quais linhas mudam comportamento. Cada linha precisa ser lida com o mesmo cuidado, e o cuidado acaba antes do diff.

Tem ainda o efeito social. Um pull request enorme representa dias de trabalho de alguém. Pedir para refazer parece desfeita, e o revisor aprova para não travar o colega. O tamanho vira pressão.

Os três tipos de mudança e por que separar

  • Mecânica: formatação, renomeação feita por ferramenta, ordenação de imports. Não muda comportamento. O revisor confere que foi gerada pela ferramenta e aprova em minutos.
  • Refatoração: extrair função, mover arquivo, trocar estrutura interna. Não deveria mudar comportamento, e os testes existentes devem passar sem alteração. O revisor procura mudança acidental.
  • Comportamento: a regra nova de frete. É aqui que o revisor precisa de toda a atenção, e é aqui que ela deve ser gasta.

Se um pull request de refatoração precisa alterar teste existente para passar, ele mudou comportamento. Essa é uma boa pergunta para fazer em toda revisão.

Como quebrar o pull request do frete

  1. Formatação primeiro, a partir da branch principal. Um commit só, gerado pela ferramenta, com a mensagem dizendo isso.
  2. Refatoração em seguida, empilhada sobre a formatação. A extração da função calcularFrete de dentro do controlador, sem mudar a regra.
  3. Comportamento por último, empilhado sobre a refatoração. As doze linhas da regra nova, com testes para cada região.

Os comandos para o primeiro pull request:

git switch main
git pull
git switch -c chore/formatar-checkout
npx prettier --write src/checkout
git commit -am "Formata src/checkout com Prettier, sem mudança de comportamento"
git push -u origin chore/formatar-checkout
gh pr create --base main --title "Formata src/checkout com Prettier" --body "Só formatação gerada pelo Prettier. Nenhuma mudança de lógica."

Para o segundo, que depende do primeiro, a branch nasce da branch de formatação e o pull request aponta para ela como base. O diff mostrado ao revisor contém só a refatoração.

git switch -c refactor/extrair-calculo-frete
# extraia a função e rode os testes existentes sem alterá-los
git commit -am "Extrai calcularFrete do controlador, sem mudar a regra"
git push -u origin refactor/extrair-calculo-frete
gh pr create --base chore/formatar-checkout --title "Extrai calcularFrete" --body "Refatoração. Testes existentes passam sem alteração."

Se você já tem a branch grande pronta, traga o arquivo de lá em vez de reescrever. O comando git restore --source=feature/frete-novo -- src/checkout/frete.js copia a versão do arquivo daquela branch para a sua árvore de trabalho, e git add -p deixa você escolher, trecho por trecho, o que entra em cada commit.

Quando o primeiro pull request é integrado, o GitHub costuma oferecer a troca da base do seguinte para a branch principal. Confira que o diff do segundo continua mostrando só o que é dele antes de pedir revisão.

Formatação que não suja o histórico

Commits de formatação atrapalham o git blame, porque todas as linhas parecem ter sido alteradas por quem rodou a ferramenta. Coloque o hash do commit de formatação num arquivo .git-blame-ignore-revs na raiz do repositório. O GitHub respeita esse arquivo na tela de blame, e localmente você configura com git config blame.ignoreRevsFile .git-blame-ignore-revs.

A descrição que faz a revisão funcionar

O revisor não estava na sua cabeça quando você escreveu o código. A descrição é o que diz para ele onde olhar. Um modelo em .github/pull_request_template.md faz o GitHub preencher a descrição automaticamente em todo pull request novo:

## O que muda para quem usa
Uma ou duas frases. Ex.: o frete para CEPs fora da região Sudeste passa a usar a tabela nova.

## Tipo de mudança
- [ ] Mecânica (formatação, renomeação por ferramenta)
- [ ] Refatoração (sem mudança de comportamento)
- [ ] Comportamento

## Como testar
1. Carrinho com CEP 01310-100: frete esperado R$ 18,90.
2. Carrinho com CEP 69005-000: frete esperado R$ 42,00.

## Onde olhar com mais cuidado
src/checkout/frete.js, função calcularFrete.

## Como reverter
Reverter este pull request. Sem migração de banco.

"Como testar" com valores esperados é a parte que mais muda a revisão. O revisor pode rodar os dois casos em cinco minutos, e se o resultado não bater, achou o bug antes da produção. No caso do frete em dobro, o segundo teste teria mostrado R$ 84,00.

Desfazer um pull request pequeno também é simples. Se algo der errado depois da integração, reverter as doze linhas do frete não leva junto a formatação de trinta arquivos. E se o erro estiver no último commit ainda local, o guia sobre desfazer o último commit resolve antes de chegar à revisão.

Quando a tarefa é grande de verdade

Algumas mudanças são grandes por natureza: um fluxo de pagamento novo, uma troca de biblioteca. Mesmo nesses casos, entregue em passos que já funcionam sozinhos. Primeiro a estrutura nova sem uso, depois um caminho usando a estrutura nova atrás de uma flag, depois a migração dos outros caminhos, por fim a remoção do código antigo. Cada passo é revisável, integrável e reversível, e nenhum depende de "só funciona quando tudo estiver pronto".

Pull requests menores também combinam com uma janela de deploy bem definida: mudança pequena cabe na janela e volta rápido se der errado.

Erros comuns

  • Rodar o formatador na pasta inteira junto com a mudança. O diff de lógica se perde no meio de centenas de linhas de espaçamento.
  • Refatorar e mudar regra no mesmo commit. O revisor não consegue saber se a diferença de comportamento foi intencional.
  • Descrição "ajustes no frete". Sem dizer o que testar, o revisor só lê, e ler não pega bug de regra.
  • Empilhar pull requests com a base errada. O segundo aparece com o diff do primeiro junto, e a revisão volta a ser grande.
  • Aprovar por cansaço. O "ok" no diff de mil linhas dá a impressão de revisão e nenhuma de suas garantias.
  • Ignorar o histórico de blame. Sem o arquivo de revisões ignoradas, a formatação esconde quem escreveu cada linha.

Checklist

  • Cada pull request tem um tipo só de mudança.
  • O de comportamento tem, idealmente, poucas centenas de linhas ou menos.
  • A descrição diz o que muda para quem usa, em uma ou duas frases.
  • "Como testar" traz casos com valores esperados.
  • Pull requests dependentes estão empilhados com a base correta.
  • Commits de formatação estão no .git-blame-ignore-revs.
  • O repositório tem um modelo de descrição de pull request.

Abra o último pull request que você aprovou: você consegue explicar, em quatro linhas, o que ele muda e como testar? Se não consegue, ele ainda era dois pedidos.

Avalie este artigo

Referência: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/getting-started/about-pull-requests