Comparação entre Unified Diff e Split Diff
(matklad.github.io)- Mudanças pequenas podem ser analisadas suficientemente bem tanto com unified diff quanto com split diff, mas, em mudanças grandes e complexas, uma revisão baseada apenas na leitura do diff encontra limites
- Mudanças grandes devem ser exploradas e verificadas com base no estado completo da base de código em um determinado momento, como se você estivesse modificando o código real, e não apenas olhando o histórico de alterações
- A tela ideal de revisão deveria mostrar lado a lado o código atualmente no disco e o unified diff daquela área, permitindo acompanhar ao mesmo tempo o contexto do código e o conteúdo alterado
- Como o suporte das ferramentas existentes é insuficiente, usa-se um fluxo de trabalho de baixa tecnologia: fazer checkout do PR localmente, remover os commits e deixar apenas as alterações
- Com essa abordagem, é possível navegar pelos arquivos alterados e marcar hunks revisados, mas a posição atual no editor e o diff não são sincronizados automaticamente, então ainda sobra manipulação manual
Revisar o estado atual do código, não o diff
- Tanto split diff quanto unified diff funcionam bem para mudanças pequenas e simples
- Quando a mudança fica maior e mais complexa, simplesmente ler o diff deixa de ser suficiente
- Em mudanças grandes, é preciso examinar adequadamente a base de código em um determinado momento
- Prestando atenção às áreas alteradas recentemente, mas basicamente encarando como uma revisão de código comum
- É preciso executar os testes
- É preciso usar recursos de navegação do editor, como goto definition
- É preciso aplicar mudanças locais para verificar se havia outras formas possíveis de escrever aquilo
- É preciso procurar partes que deveriam ter sido alteradas em um contexto mais amplo
- É preciso conseguir encontrar pontos estranhos na base de código atual, independentemente do histórico de alterações
Tela de revisão desejada e workaround local
- A visualização de diff ideal mostraria, à esquerda, o estado atual do código e, à direita, o unified diff da área de código atualmente visível
- O código à esquerda deveria ser igual ao estado atual em disco
- As alterações seriam indicadas de forma sutil na margem
- O diff à direita deveria corresponder à área de código atualmente visível à esquerda
- Atualmente, o suporte das ferramentas a esse formato de revisão é insuficiente
- O fluxo de trabalho real consiste em fazer checkout do PR localmente usando o gpr script
- Na etapa final, os commits do PR são removidos e apenas as alterações permanecem
- Nesse estado, o fluxo de trabalho de staging e commit é aproveitado para a revisão de código
- Com o edamagit, é possível ver a lista de arquivos alterados e navegar para a próxima/anterior alteração no editor
- Também é possível usar a staging area para marcar hunks já revisados
- O incômodo restante é que não há sincronização automática entre o magit status buffer e o arquivo atualmente aberto no editor
- Para ver o arquivo atual e o diff lado a lado, é preciso abrir o diff manualmente e rolar diretamente até a posição que se está vendo
- Para fazer uma revisão tão próxima do código, hoje é necessário criar suas próprias ferramentas provisórias
- O principal objetivo de uma revisão de código não é necessariamente apenas examinar o código; como leitura relacionada, também há Two Kinds of Code Review
1 comentários
Opiniões do Hacker News
Dizer que, em mudanças grandes, você não quer fazer uma revisão de diff, mas sim revisar corretamente a base de código em um determinado momento e prestar atenção às áreas alteradas recentemente, pessoalmente, não parece combinar com uma revisão de código comum
Depende da equipe e do ticket, mas, se o revisor não estiver em um papel como o de principal engineer, responsável principalmente pelo código como um todo, em geral vejo a revisão de código mais como uma simples verificação de consistência feita por um segundo par de olhos
Se os commits do PR estiverem bem organizados ou tiverem sido squashados, também fica relativamente fácil revisar as mudanças incrementais separadamente
No fim, a reclamação parece mais uma diferença entre tipos de revisão. Há a revisão comum de verificação de consistência e a revisão pre-merge profunda de arquitetura/funcionalidade, e a revisão de código em ferramentas web parece, em sua maioria, mais próxima da primeira
Uma boa revisão de código começa com um bom PR. O PR deve explicar o objetivo e como esse objetivo foi alcançado
O primeiro item de uma boa revisão é verificar se o código cumpre o objetivo descrito; o segundo, se há testes que validem a funcionalidade alegada; o terceiro, se segue as convenções arquiteturais do sistema existente; o quarto, se o estilo está de acordo com o esperado; e, por fim, se há sugestões para melhorar o uso da API
Revisões de código que fazem apenas verificação de consistência são pouco úteis, pois isso é algo que o CI poderia fazer. Revisão de código é um processo humano, então deve aproveitar ao máximo o que só humanos conseguem fazer e deixar o restante para as máquinas
Nos itens acima, está implícita a pergunta: “Esta mudança será um bom exemplo para contribuições futuras?”
Sempre fiz revisão de código com uma perspectiva de testador. Compilo e executo o código para ver se ele funciona como pretendido, procuro pontos fracos e tento quebrá-lo
O diff não é o fim, é o começo. É apenas uma pista de onde cutucar o código; no fim, a prova vem do comportamento real
Alguém poderia dizer: “isso é trabalho de testador, não de engenheiro”, mas quem poderia testar melhor do que alguém que entende o código e sabe onde olhar?
Sei que essa postura é minoritária. Muitos engenheiros parecem ter uma resistência estranhamente forte a executar o código diretamente, e até a usar um debugger. Constroem uma infraestrutura enorme para automatizar tudo, mas automação, no fim, também é apenas código que pode ter bugs. Quem testa os testes, Quis custodiet ipsos custodes?
A revisão de código é a última linha de defesa, excluindo o CI, mas nas equipes em que trabalhei a direção geral introduzida pelo PR já tinha sido discutida com várias pessoas antes desse ponto. Pode ser pair programming, explicações aprofundadas, conversas no café, qualquer forma adequada à complexidade
Assim, o PR vira um espaço para revisar as táticas que implementam a estratégia, não a estratégia em si, como separar um novo módulo, introduzir uma grande dependência ou retrabalhar a superfície de uma API
Se você apenas verifica a consistência de uma parte do código isoladamente, sem comunicação prévia, há o risco de fragmentar a posse do código. Acaba virando algo como: “O que aquele módulo faz?” “Não sei, pergunte ao Bob”
Separadamente disso, seja qual for o modo de revisão, gostei da ideia de visualização de diff proposta pelo autor
É difícil fazer qualquer revisão com um bloco misturando linhas vermelhas e verdes. Não é uma visualização útil do código e, no mínimo, não é adequada como visualização padrão
Ainda assim, como é trabalho, a gente vai empurrando, mas concordo 100% com o autor que não é uma visualização muito útil
Sem um processo básico para lidar com defeitos, funcionalidades etc., cada engenheiro precisa inventar, a cada vez, como lançar uma funcionalidade por meio de colaboração
A maioria acaba fazendo quase nada, e a revisão de código se torna a única colaboração feita em um nível de detalhe significativo sobre o assunto. Por isso, tudo que poderia ser uma intervenção acaba sendo enfiado em um único momento
Como terceira, ou quarta, opção, também vale mencionar o difftastic. Ele usa um diff “estrutural” em vez de diff por linha, destacando mudanças de forma mais granular
https://github.com/Wilfred/difftastic
Pelo que me lembro, às vezes ele deixava cerca de 35% dos diffs mais fáceis de ler; em geral, em 60% não fazia diferença; e, raramente, em 5% os tornava mais difíceis de ler
Usei há alguns anos, então não me lembro bem dos problemas específicos. O único motivo de eu ter parado foi que comecei a usar magit para Git diff
git diff --word-difftambém tem uma funcionalidade parecida e é bem boaAo revisar no Vim, a sensação é parecida
Tenho um pequeno script que abre o PR e, para os arquivos alterados, faz algo mais ou menos como
vimdiff <(git show baseref:file) file. As abas do Vim são, na prática, views, então é bom poder manter buffers individuais abertos ao mesmo tempo em estados diferentesNa view padrão, o scroll lock funciona como esperado e, se necessário, é possível evitá-lo em uma aba separada
[ce]cnavegam entre hunks com base no conjunto de alterações existente no PR, não no diretório de trabalhoCom
dpedg, dá para fazer push/pull de um hunk para o buffer de diff somente leitura e marcá-lo como “concluído”; com isso, ele fica oculto entre as alterações destacadas do buffer ativoUma alteração feita no buffer ativo pode ser commitada imediatamente ou enviada como comentário
Recursos normais do editor, como go to definition, integração com build e documentação em pop-up, continuam funcionando no estado atual da árvore
Esse método permite ver as alterações do PR em relação ao branch-base e, ao mesmo tempo, manter o diretório de trabalho limpo. Parece muito melhor do que a solução do matklad, que vê as alterações deixando o diretório de trabalho sujo
No ambiente em que trabalho, isso é especialmente bom porque as alterações muitas vezes são adicionadas como commits
--fixup, e a ferramenta as recombina no momento do merge de modo que, por meio de uma chamada agit-interpret-trailers, o autor do commit de fixup seja atribuído ao commit original. Ela também extrai comentários de texto do PR e, quando apropriado, adiciona um trailerReviewed-by, ou um trailerAcked-byquando equivale a um +1Em bases de código muito grandes e complexas, com muitos engenheiros envolvidos, isso é um grande problema
Code review é difícil porque o diff sempre parece plausível, os testes sempre passam e o básico está sempre verificado
Mas, mesmo quando a alteração parece razoável, muitas vezes ela está errada na prática
Uma única alteração ruim que pareça plausível pode fazer toda a arquitetura derivar
Como sempre, estritamente falando, isso não é um problema de ferramenta; está mais para um problema de cultura e compartilhamento de conhecimento. Ferramentas melhores, sozinhas, não resolvem
Ao fazer code review, é raro encontrar o bug em si. Em vez disso, olho se existem testes que detectariam esse tipo de bug
Acho que estou deixando passar alguma coisa. O autor diz que split diff não funciona para ele, mas não explica por quê
Um diff ideal pareceria quase igual a um split diff com o contexto duplicado removido. Em vez de o contexto se repetir dos dois lados, ele ficaria só à esquerda
Fico me perguntando qual é a utilidade de remover o contexto duplicado do painel da direita
Nos editores/IDEs que usei, o suporte a esses recursos dentro de um diff costuma ser básico. Go to definition às vezes só funciona dentro do mesmo arquivo, autocomplete geralmente não funciona bem para itens recém-adicionados, e muitas vezes não há recursos de refatoração
Pelo que entendo, o autor quer inverter o diff. Em vez de olhar a alteração em si, quer olhar o código e verificar se há mudanças relacionadas a ele
O texto não tem muitos detalhes, mas parece que ele quer navegar até implementações ou pontos de chamada e verificar se há alterações adequadas
O destaque de diff pode distrair mais do que parece e, quando desligado, às vezes a legibilidade melhora bastante. Eu também às vezes desligo isso no visualizador de diff para olhar o código com olhos novos, mas não há opção de mostrar o diff unificado junto em um split pane
Ao pressionar
.no GitHub, você entra em uma IDE completa dentro do navegadorAchei muito útil para revisão. Em vez de ver só fragmentos, dá para ver as alterações dentro do contexto do arquivo inteiro, o que ajuda muito mais a encontrar problemas sutis de design
Aliás, esse atalho também funciona no GitLab
Sinto falta de alguns recursos do p4merge. Por exemplo, há um recurso que, no espaço entre os split diffs, mostra qual parte de um arquivo corresponde a qual parte do outro, exibindo também o impacto do diff nos dois lados: https://www.perforce.com/manuals/p4merge/Content/P4Merge/dif...
Também gosto do recurso de merge em 3 vias: https://www.perforce.com/manuals/p4merge/Content/P4Merge/dif...
Esses recursos não chegaram às ferramentas de diff baseadas na web mais usadas. Acho que o diff em 3 vias é uma boa maneira de mostrar o resultado de um merge difícil
Se você usa GitHub, basta pressionar
.na página do pull request em github.com ou mudar o domínio paragithub.devIsso abre o VSCode no navegador, e o pull request é exibido em uma view de diff
A vantagem é que dá para ver o arquivo inteiro e, como o algoritmo de diff é diferente do github.com, às vezes fica mais legível em diffs complexos
A view de diff lado a lado que mostra comentários do PR em uma lista fora das linhas pode ser muito útil, mas é uma pena precisar abrir uma instância inteira do VSCode no navegador
Na verdade, isso é tão útil que o GitHub já teve uma split view assim antigamente, sem precisar empurrar as pessoas para uma salada de buzzwords de ambiente de desenvolvimento AI/ML/crypto/blockchain/cloud-enabled
A ideia é boa, mas a execução é péssima. Talvez isso também resuma o GitHub como um todo hoje em dia
Meld se encaixa muito bem nesse tipo de uso
https://meldmerge.org/
Minha ferramenta favorita é o diff2html-cli: https://diff2html.xyz/
https://www.npmjs.com/package/diff2html-cli
Permite visualizar diffs em HTML e suporta os formatos side-by-side e unified