A revisão de código pode ser melhor
(tigerbeetle.com)- Muitos desenvolvedores estão insatisfeitos com a experiência de revisão de código no GitHub e vêm tentando novas abordagens para melhorá-la
- Uma ferramenta experimental chamada
git-reviewfoi projetada para tratar a revisão de código localmente, junto com o código, em vez de depender de uma interface web no navegador - A revisão é gerenciada como um único commit, deixando comentários de revisão como anotações no código, e revisor e autor vão editando esse commit em conjunto
- No entanto, quando o código é alterado ou rebaseado durante a revisão, surgem incômodos com resolução de conflitos e uso de
--force-with-lease, o que impediu maior adoção - No fim, houve um retorno à revisão baseada na web, mas a ideia de incluir o estado da revisão diretamente no repositório continua atraente, e ainda há possibilidade de surgirem alternativas melhores com futuras melhorias no Git, como a adoção de um Change-Id no estilo Gerrit
Percepção dos problemas nos sistemas de revisão de código
- Atualmente, muita gente está insatisfeita com o processo de revisão de código do GitHub
- Os principais problemas incluem a falta de suporte para pull requests empilhados e revisão de interdiff, além de que
- o estado da revisão não é armazenado dentro do repositório
- a revisão exige uma interface web remota em primeiro lugar
- Os problemas que eu vejo são a falta de descentralização da revisão e a ineficiência da interface
Comparação entre fluxo de escrita de código e fluxo de revisão
- As pessoas escrevem código localmente usando um editor
- É um ambiente com baixa latência de memória e NVMe, otimizado para o fluxo de trabalho particular de cada pessoa
- Também preferem revisar código fazendo pull da branch de origem localmente
- Com ferramentas como Magit, é possível explorar não só o diff, mas também todo o contexto do código
- Dá para aproveitar um ambiente de desenvolvimento poderoso, com execução de testes, navegação para definições, tentativas de refatoração etc.
- Em contrapartida, para deixar feedback em uma PR, é preciso ir para o navegador e usar uma interface web lenta, que em diffs grandes chega a ter atraso até na digitação
Interface e estrutura de armazenamento ideais para revisão de código
- Na prática, o mais natural é deixar comentários inline no código ou até editar o próprio código diretamente
// CR(matklad): Hm, this check seems imprecise to me. // Shouldn't we compare `replica.view` instead of `header.view` here? if (header.view != view) return; - Quando os dados ficam em um banco de dados remoto, e não no repositório git local, também surgem problemas de latência e vendor lock-in
A ideia do git-review e a experiência prática
- A ideia do
git-reviewé a seguinte:- a revisão de código acontece em um único commit no topo da branch da PR
- nesse commit, são adicionados comentários no código com marcadores especiais
- revisor e autor vão alternando edições nesse commit, colaborando com base em push --force-with-lease
- quando todos os comentários são marcados como resolvidos (
//? resolved), um commit de revert é adicionado ao final da revisão para preservar o registro
- A ideia é simples e prática, mas na realidade surgiram os seguintes problemas
- quando o código é alterado durante a revisão, há conflitos frequentes entre os comentários e commits inferiores ou novos commits
- o processo de force-push aumenta o atrito da colaboração e a complexidade do trabalho
- fica difícil gerenciar a inconsistência entre o histórico de mudanças do código e o andamento da revisão, assim como os conflitos de merge
Novas mudanças e possibilidades futuras
- No futuro, pode haver adoção de um Change-Id no estilo Gerrit no Git upstream
- Isso deve facilitar o rastreamento do histórico de alterações por commit e ampliar o suporte a revisão de interdiff
- Mas há expectativa de alguns conflitos com a abordagem do
git-review - Com a nova estrutura de Change-Id, talvez se tornem possíveis abordagens diferentes, como adicionar comentários de revisão ao próprio commit
Conclusão e apresentação de sistemas relevantes
- No momento, acabou havendo um retorno à revisão de código baseada em interface web
- A necessidade de uma solução melhor continua existindo
- Apresentação de sistemas e ferramentas relacionadas que valem a referência
- Fossil: sistema de SCM que guarda todas as informações dentro do repositório
- NoteDb: integra ao git o histórico de armazenamento do estado de revisão do Gerrit
- git-bug: armazena informações de issues no git
- git-appraise: mantém informações de revisão no próprio git
- prr: implementa uma interface de revisão dentro do editor, integrada à API do GitHub
- How Jane Street Does Code Review: apresenta um exemplo de uma realidade melhor
- git-pr: projeto que substitui todo o fluxo de trabalho de PR por funcionalidades nativas do git
Encerramento
- Ainda não existe uma solução perfeita, e as tentativas por uma experiência melhor para desenvolvedores continuam
- Há grande expectativa em relação aos próximos avanços
2 comentários
Não sei se a abordagem do
git-reviewé boa, mas concordo que review de PR baseado em GitHub é horrível..Opinião do Hacker News
Uma coisa que me incomoda em code review há muito tempo é que o feedback realmente útil — e não só observações de preferência pessoal — quase sempre chega tarde demais. A única (ou rara) conclusão da review acaba sendo algo como "tem que refazer tudo com um design novo" ou "nem precisava ter feito esse trabalho desde o começo". Parece que code review é o único momento em que todas as partes interessadas realmente participam e pensam seriamente sobre a mudança. Pode até ter havido alguma discussão em reunião ou ticket do Jira, mas muitas vezes alguém de outro time ou departamento só fica sabendo dessa mudança quando recebe a notificação da code review. Eu mesmo, quando outro time implementa alguma coisa estranha, muitas vezes só descubro pela notificação da code review. É irrealista esperar que todo mundo acompanhe tudo antecipadamente. Na faculdade, nos anos 90, a gente fazia design review, mas nunca vi isso de verdade no mercado. E também não acho que design review garanta capturar todos os problemas antes.
No mundo da engenharia de software, existe pouca engenharia de fato. Também tem o lado de que a indústria não aceita bem a lentidão de um processo de engenharia de verdade. A maior parte do software não é crítica, e bugs ou erros podem ser corrigidos depois. O nível de risco e a possibilidade de corrigir depois são diferentes de áreas como pontes, fábricas ou motores de avião, onde falhar não é uma opção.
Nosso time é um grupo pequeno de 4 a 6 devs. Quando surge um rascunho mental de uma funcionalidade nova, eu já converso com os colegas imediatamente. Como todo mundo faz isso, a code review acaba focando mais em coisas pequenas, como code smells, e a arquitetura geral normalmente já foi decidida por 2 ou 3 pessoas antes. Quando o pessoal não concorda com o código, acaba evitando mexer no código uns dos outros, e a situação piora. Acho que isso continua funcionando mesmo em escalas maiores, desde que a responsabilidade seja bem compartilhada.
O fato de todas as partes interessadas só participarem na fase de code review não é um problema do Git ou do sistema de controle de versão, e sim da organização. O problema é não conseguir compartilhar o contexto da criação do PR, as discussões do ticket e o processo de decisão. Isso é um exemplo de organização disfuncional; é como culpar o processo editorial porque todo mundo só se reúne para se envolver de verdade quando o livro impresso já saiu.
Na nossa organização, toda decisão de design fundamental exige um RFC. O que conta como decisão de design fundamental é algo que o time avalia de forma autônoma o tempo todo. Se na fase de epic do Jira ainda não estiver definido como a implementação vai funcionar em detalhe, a primeira tarefa já é escrever um RFC. O RFC pode ser só para o nosso time ou para todo o time de software; neste segundo caso, todo mundo pode ler e comentar antes da reunião quinzenal. Dá trabalho, mas ainda é muito melhor do que trabalhar num lugar sem um processo colaborativo de design baseado em RFC.
Pela minha experiência, concordo com a questão da design review. Antes fazíamos documentos formais de design e revisões, mas migramos para prototipação e design iterativo. Na fase de design, a gente frequentemente deixava passar detalhes importantes e, como já tinha investido muito tempo nisso, depois acabava relevando. Também era ineficiente juntar o time inteiro para revisar, então no fim os problemas eram encontrados durante a code review. Muita gente também escreve mal documentos de design ou não tem motivação para isso. No fim, com mais de 5 pessoas, esse tipo de ineficiência é inevitável. O ideal é um ambiente com o PO, os principais usuários e uns 5 desenvolvedores.
Achei muito interessante ver no HN um post sobre stacked pull requests. Quando a graphite.dev começou, era comum que quem não tinha passado por Facebook ou Google nem conhecesse esse fluxo. É curioso ver como as tendências de code review mudaram tão rápido nos últimos 3 ou 4 anos.
Como usuário de arcanist desde antes do mercurial, ainda faço bastante propaganda do Graphite para times que continuam sofrendo com PRs grandes e merge commits. Fiquei especialmente impressionado com a ousadia e com o resultado de terem viabilizado a integração com PRs. Seria ótimo se o Graphite tivesse um modo mais prescritivo, que inicializasse o repositório com uma configuração hardcore e pudesse assumir restrições mais fortes.
Apesar de o Graphite ser uma solução excelente, ele é bem caro, e é difícil convencer quem toma a decisão de compra. Espero que uma ferramenta tão boa quanto o Graphite vire open source ou seja incorporada ao GitHub.
Às vezes eu sinto falta do workflow do fig.
Depois do incidente de segurança recente no CodeRabbit, fiquei relutante em testar ferramentas novas que integrem LLMs ao codebase. Uma experiência nova e empolgante pode virar facilmente uma dor de cabeça de segurança.
Stacked pull requests essencialmente adicionam uma complexidade desnecessária. Mudanças pequenas e frequentes são uma prática de desenvolvimento muito melhor. Trunk-based development e continuous integration combinam bem mais com esse objetivo.
Parece haver cada vez mais consenso entre desenvolvedores sobre “como uma ferramenta de review deveria ser”. Agora o ponto mais importante é quais organizações e atores vão conseguir tornar isso realmente aplicável na prática e sustentável. A adoção recente de git change-id é um avanço muito positivo (obrigado a jj, git butler, gerrit etc.). Graphite e GitHub focam em soluções voltadas só para os próprios usuários, então não seguem uma direção aberta para todos. E várias ferramentas de linha de comando baseadas em cliente também não têm tanto impacto assim. O que realmente precisa existir é:
Minha maior reclamação com o GitHub é que o app é lento demais. Lento num nível de travar aba do navegador. O Azure DevOps foi, de longe, a melhor ferramenta de code review que já usei.
Já usei Azure DevOps para desenvolver .NET em ambiente Microsoft, e realmente é uma ferramenta que combina muito bem com o ecossistema .NET.
Fico curioso se você já usou GitLab a sério. Entre os big 4, é o que eu mais gosto.
Queria entender o que exatamente te agradou tanto no DevOps. Uso todo dia e acho parecido com o GitHub. Às vezes até sinto falta da função do GitHub de aplicar automaticamente alterações sugeridas.
O uso massivo de JavaScript junto com a pressão por releases rápidas cria esse tipo de ambiente lento. Ainda assim, é melhor do que Atlassian.
A ideia de usar o git diretamente para code review é atraente. É conveniente poder mexer nas mudanças localmente. Não entendo por que a review precisaria estar presa a um único commit — uma abordagem em que o revisor faz commits com seus próprios comentários/correções diretamente no branch do PR também me parece interessante. É um híbrido entre o github flow tradicional e o fluxo de mailing list/patches do Linux.
Fico me perguntando se o PR do GitHub é somente leitura. Já tive experiência com alguém do time propondo uma correção direto num comentário de "suggestion" e aquilo sendo aplicado ao commit com um clique.
Essa ideia de que o commit de review precisa ser único é estranha. Se várias pessoas revisarem, também surgem problemas de edição concorrente, e isso não combina com o próprio código. O natural seria que a review também fosse feita em branches separados por pessoa, e no fim tudo fosse squash & rebase. Se a discussão se alongar, até commits de comentário poderiam ficar encadeados entre si. O importante é que esses dados fiquem guardados em algum lugar fora da main branch.
Quando faço code review, prefiro puxar para um branch local, dar soft-reset e olhar como se eu mesmo tivesse escrito aquilo. Se os commits não estiverem bem divididos, cada colaborador acaba tendo que refazê-los mentalmente para revisar, o que é ineficiente. Quando o escopo da review é grande demais, é difícil dizer que alguém realmente entendeu tudo. O todo não é simplesmente a soma das partes.
Compartilho um exemplo de função shell para facilitar code review. Ela presume uma árvore limpa, faz checkout de um branch de review predefinido, abre o diff no nvim e depois automatiza a limpeza do branch quando o trabalho termina.
Saber fazer bons commits e bons PRs a partir deles é uma habilidade tão importante quanto escrever código bem. Mas, na prática, há mais desenvolvedores do que se imagina que são fracos em dividir PRs e escrever mensagens de commit.
Em times de trabalho paralelo em ciência de dados, usamos 2 ou 3 branches em checkout para code review.
Para review de PR, uso https://github.com/sindrets/diffview.nvim no Neovim. A interface é parecida com o diff do vscode, mas aproveita o modo diff do vim. Para reviews leves,
git log -p --function-contexttambém é útil.Tenho interesse nesse modelo em que uma pessoa faz o primeiro draft e outra pessoa lapida e faz o merge. Queria saber se alguém aqui já trabalhou assim.
Concordo com esse estilo em que mais de 90% da codificação real fica com uma pessoa, enquanto o revisor assume a responsabilidade pela versão final e pelo merge. Num emprego anterior, o revisor sempre era responsável por fazer o merge, e, se a mudança fosse grande, só repassava os comentários. Isso é muito mais eficiente do que ficar pedindo para alguém clicar em aprovar.
Quando você participa da escrita do código que revisa, sente que consegue dar um feedback muito mais profundo. Esse modelo ajuda a enxergar o código como "nosso código", e não "meu código/seu código". Combina bem com culturas iterativas e colaborativas, como TDD.
Esse modelo se parece com pair programming assíncrono.
Conheço pessoas que usam pair programming dentro de trunk-based development. As duas escrevem o código juntas, e, se estiver OK, já fazem merge direto na main; se os testes passarem, vai para produção na hora. Na prática, funciona bem.
Uso a extensão de GitHub Pull Request no VSCode para revisar localmente, e é bem conveniente. Dá para comentar e revisar direto no editor.
Na web do GitHub, quando há muitos arquivos alterados, ele começa a esconder arquivos, mas no VSCode dá para navegar livremente e a experiência fica muito melhor. Como isso já existe na combinação VSCode/GitHub, imagino que possa ser expandido para outros editores também.
Está melhorando, sem dúvida, mas ainda existe um vendor lock-in forte entre GitHub e VSCode.
Na web do GitHub, abrir um PR e apertar “.” para ir direto para o VSCode web também é uma experiência muito melhor.
Fiquei muito feliz em ouvir que o Git vai ganhar change ID como recurso de primeira classe! É parecido com o revision tracking dos diffs do Phabricator no Facebook. Queria links para saber mais.
Também queria mencionar o Sourcehut. Ele mantém o fluxo clássico de trocar patches/issues/bugs/discussões por e-mail e integra isso com mailing list e CI. O Drew Devault também oferece recursos sobre como enviar e receber patches em git-send-email.io e git-am.io.