3 pontos por GN⁺ 2024-09-12 | 1 comentários | Compartilhar no WhatsApp
  • Revisões de PR no estilo GitHub facilitam continuar empilhando commits de correção sobre a alteração original, criando o problema de misturar o processo de revisão com o histórico final do código
  • Uma boa série de patches deve permitir que commits logicamente separados, como refatoração, adição de uma nova API e migração de usuários existentes, sejam lidos em ordem
  • Quando commits como “fix review” ou "minor" se acumulam, a intenção da alteração fica menos clara, e aumenta o custo de encontrar a causa de um problema com git blame e git bisect
  • A revisão com interdiff publica uma nova versão da série de patches e compara as diferenças por commit com git range-diff main..v1 main..v2
  • O revisor pode verificar apenas um incremento de 50 linhas em vez de reler todas as 500 linhas, e o autor consegue manter a estrutura final de commits sem commits ruidosos como “address review”

Ferramentas de revisão de código e ponto de partida

  • Gerrit Code Review é uma ferramenta open source de revisão de código que funciona junto com repositórios Git e dá suporte ao fluxo de enviar patches, ter o código revisado por outras pessoas e receber comentários
  • Há várias formas de revisão de código: Gerrit, GitHub e Phabricator, mas também enviar arquivos .patch em um bug tracker, usar git send-email ou conduzir a implementação após longas discussões com colegas
  • A linguagem de programação Go usa Gerrit; KDE e LLVM usaram Phabricator; e o Jujutsu já considerou usar Gerrit
  • O kernel Linux usa um fluxo de trabalho baseado em e-mail de uma forma particular, e muitos projetos escolhem o GitHub por ser fácil de usar e ter baixo custo inicial
  • O ponto central é que as ferramentas não oferecem o mesmo modelo de revisão, e a experiência padrão do GitHub nem sempre é suficiente

Condições de uma boa série de patches

  • Uma série de patches ideal apresenta uma alteração grande dividida em etapas lógicas
    • Primeiro, faz uma limpeza de código encontrada incidentalmente
    • Em seguida, adiciona uma nova API
    • Por fim, migra os usuários da API existente para a nova API
  • Cada patch deve ter um motivo independente, e, quando aplicado em ordem, deve deixar visível como o código evolui gradualmente
  • Essa estrutura é vantajosa não só para autor e revisor, mas também para mantenedores que, mais tarde, precisem encontrar o motivo de uma alteração com blame ou bisect
  • O tamanho real dos patches varia conforme a situação
    • Pode não haver refatoração
    • Pode ter 100 linhas
    • Pode chegar a 500 linhas se uma mudança na API central exigir corrigir todos os pontos de chamada

O histórico misturado criado pela “diff soup” no estilo GitHub

  • Ao incorporar comentários de revisão, o GitHub induz, de forma explícita e implícita, o fluxo de adicionar novos commits sobre os commits originais
  • No exemplo, depois dos 3 commits originais, acumulam-se, em sequência, commits para incorporar as revisões de Alice e Bob, adicionar testes e fazer uma alteração de implementação "minor"
  • Como o grafo de commits preserva apenas relações de pai e filho, não fica claro qual problema de qual commit original foi corrigido por um commit de ajuste da revisão
    • Se um único commit “fix review” mexe em vários commits originais ao mesmo tempo, o modelo conceitual fica ainda mais complexo
    • É possível fazer com que cada commit de correção incorpore exatamente um apontamento, mas isso aumenta a quantidade de commits de fixup
  • Essa forma de misturar, na mesma tigela, um grande conjunto de alterações e correções de revisão é a “diff soup

O sinal de git blame e git bisect fica menos claro

  • Como git blame funciona linha a linha, se um commit que incorpora revisão altera apenas uma pequena parte de uma linha existente, a origem dessa linha pode passar a aparecer como um commit do tipo "fix alice review"
  • Para encontrar o motivo real da alteração, passa a ser necessária uma arqueologia de commits, seguindo novamente o commit original e o commit de correção da revisão
  • Também no git bisect, pode ficar incerto se o commit "minor" é a causa real ou se é um bug que já existia desde que a nova API foi adicionada e só apareceu depois
  • Se a nova API oferece um grande ganho de desempenho, reverter a mudança pode não ser desejável ou sequer possível
  • Se a regressão aparece semanas depois, surge o custo de ter de investigar mais a fundo, em uma situação urgente como uma reunião P1, olhando apenas para uma mensagem de commit "minor"
  • Mesmo no exemplo em que a revisão passa por apenas duas rodadas, o histórico fica complexo; em projetos open source, onde várias rodadas de revisão se repetem, o problema se torna maior

Como a UX do GitHub reforça esse fluxo

  • No GitHub, a forma mais fácil na prática de fazer revisão incremental é adicionar novos commits por cima
    • Para o revisor, é menos trabalhoso olhar um patch "fix alice review" de +/- 10 linhas do que reler 500 linhas de uma nova API
  • A aba de diff do PR, por padrão, mostra todos os commits combinados em um único diff
    • Isso se parece com ver git diff master..foo-branch
    • Não combina bem com a forma como as pessoas escrevem e leem alterações
  • A UX do GitHub é fortemente alinhada a um modelo de branches nomeadas, o que dificulta ver diferenças entre unidades que não sejam branches
  • Depois de um force push, é possível ver um diff pelo botão “Compare” na linha “Force Push” do PR
    • Mas ele mostra apenas o diff completo entre o head anterior do branch e o head atual do branch
    • Para uma comparação mais granular, é preciso encontrar manualmente os IDs dos commits e colocá-los na URL

Revisão com interdiff e git range-diff

  • A revisão com interdiff não adiciona commits de correção sobre os 3 commits originais; em vez disso, publica uma nova versão formada pelos mesmos 3 commits
  • Por exemplo, se for necessário incorporar os comentários de Bob na série de patches v1, cria-se a série v2 modificando o primeiro e o terceiro commits
  • Mesmo incorporando depois os comentários de Alice, a adição de testes e a alteração "minor" da mesma forma, o resultado final continua sendo 3 commits
  • Se os commits A, B e C de v1 evoluíram para X, Y e Z em v2, o git range-diff mostra a diferença entre os commits em cada posição
git range-diff \
  main..v1 \
  main..v2
  • Esse comando mostra o diff par a par entre os 3 commits do branch v1 e os 3 commits do branch v2
    • O primeiro commit é diff(A, X)
    • O segundo commit é diff(B, Y)
    • O terceiro commit é diff(C, Z)

Efeitos práticos da abordagem com interdiff

  • Ao contrário do GitHub, que normalmente mostra o diff do branch inteiro, como diff(main, C) ou diff(main, Z), o interdiff mostra a diferença entre versões de commits
  • O revisor pode verificar apenas as 50 linhas modificadas, sem reler a alteração de API de 500 linhas que já tinha visto
  • O autor não precisa acumular no histórico 30 commits ruidosos como “address review”
  • O git blame fica mais propenso a associar linhas ao commit original da alteração, com menos ruído
  • O git bisect fica mais propenso a apontar a regressão para uma unidade mais significativa, como o commit da nova API
  • Como resultado, a relação sinal-ruído das ferramentas básicas de diagnóstico melhora

Observações sobre estratégia de merge e rebase

  • A explicação sobre estratégias de merge de patches permanece como TODO
  • A posição é que git rebase é aceitável, exceto nos casos em que se espera que outras pessoas empilhem commits sobre um branch público
  • Sistemas de revisão com interdiff incentivam patches menores e que podem ser mesclados mais rapidamente
    • Não é preciso esperar até que todos os 5 commits estejam prontos
    • Se os 3 primeiros estão bons e os 2 últimos ainda precisam de mais trabalho, é possível mesclar apenas os 3
  • O objetivo é fazer com que as pessoas trabalhem a partir do branch main, e não de branches de longa duração, reduzindo situações em que vários branches remotos acabam entrelaçados em merges
  • Também há projetos, como o kernel Linux, que mesclam branches públicos com outros repositórios ou os publicam explicitamente sobre outros branches
    • Desenvolvedores do Linux podem usar git rebase ao criar e refinar séries de patches
    • Mas, depois de criar um branch público, como ele pode ser mesclado na árvore de alguém, eles não fazem force push; em vez disso, criam um novo branch com novos commits

1 comentários

 
GN⁺ 2024-09-12
Opiniões do Hacker News
  • No GitHub, em geral uso esse fluxo, mas a desvantagem é que dá mais trabalho para mim e não é intuitivo para os colaboradores
    Ainda assim, mantém as vantagens de o revisor poder ver apenas o diff que incorpora o próprio feedback, sem quebrar o git blame e o git bisect
    Ao incorporar feedback de revisão, faço o commit com git commit --fixup , envio o push e deixo o hash do commit fixup em uma resposta ao comentário da revisão
    Quando o PR é aprovado e está prestes a ser mesclado, executo git rebase --interactive origin/main --autosquash para juntar os commits fixup aos commits originais e, por fim, faço git push --force-with-lease e mesclo
    É preciso ter cuidado com force push antes do fim da revisão. Se fizer isso, o revisor não conseguirá ver o que foi adicionado desde a última revisão
    Dependo bastante do autocompletar do terminal, então só digitar algo como git re já me leva até o comando longo, mas é um pouco tosco; seria bom ter uma ferramenta que desse suporte e incentivasse esse fluxo. Ainda assim, se você está preso ao GitHub, isso é aceitável

    • Eu ia dizer o mesmo. O rebase interativo resolve boa parte do problema de “diff soup” de que o autor reclamou
      Só que, na prática, ele tende a ser usado por equipes de engenharia disciplinadas, que fazem questão de aprender recursos avançados do git
    • Basta colocar a configuração abaixo em ~/.gitconfig
      [rebase] autosquash = true
      Depois disso, só git rebase -i origin/main já reordena automaticamente os commits fixup/squash. É uma configuração pequena, mas melhorou muito o fluxo de trabalho
    • https://news.ycombinator.com/item?id=37086022
    • Commits fixup também são uma boa abordagem, mas pessoalmente não gosto. Acho muito mais elegante algo como o comando absorb do Sapling, que absorve automaticamente as alterações nos diffs relacionados sobre o SCCS weave interno e pede confirmação em uma UI interativa
      É uma área parecida com a necessidade de --update-refs ao fazer rebase de uma série de commits com vários branches empilhados. Por que uma pessoa deveria rastrear as relações do grafo, inserir commits manualmente e mover branches? Computadores lidam bem com grafos, então é só deixar para eles
      Também existe git absorb, mas ele não é tão robusto quanto a implementação do Sapling[1]
      O problema real não é se há rebase interativo ou não, mas, em geral, a experiência do usuário da própria ferramenta de revisão e o ciclo que ela induz. Por exemplo, commits fixup não resolvem o problema do GitHub de mostrar o diff entre pontos de referência, e, se o ponto de referência for grande, isso pode arruinar completamente a revisão. É o caso, por exemplo, quando você faz rebase sobre 10 novos commits
      Também tenho reclamações sobre a UX do próprio Git, mas o texto original é mais uma reclamação sobre o GitHub
      [1] Há um exemplo que mostra a diferença entre os algoritmos de base nesta issue do GitHub: https://github.com/martinvonz/jj/issues/170
    • Aliases do Git são realmente mágicos
  • Concordo 100% que esse método é o ideal. O jeito do GitHub é realmente horrível, e é uma tragédia que tanta gente tenha passado a aceitá-lo como normal
    Antigamente fazíamos isso com o Phabricator, e era possível até certo ponto de forma manual graças a uma macro de linha de comando que atualizava todas as revisões de uma vez. Ainda assim, seria melhor ter uma UI explícita

    • Sou o autor do texto, e usei de propósito a expressão “revisão de código em geral é uma ideia bem boa” logo no começo. Era uma das frases de venda que havia na página inicial do Phabricator antigamente :) Que saudade
  • Certo. Isso é exatamente o que eu imaginava na minha cabeça como um estilo de revisão de código de verdade, não o jeito que o GitHub faz. Fico feliz em saber que existe um nome para isso
    Além disso, seria bom se o sistema de revisão pudesse empurrar patches prontos “para fora” da revisão. Uma pequena correção de bug feita durante o trabalho em uma funcionalidade grande deveria ser um patch pequeno e independente, e tem grande chance de ser rapidamente acordada com o revisor
    Então eu gostaria de remover esse patch da série inteira, fazer cherry-pick dele para a main e fazer rebase da revisão sobre o novo HEAD. Ou então fazer rebase da série de patches mais recente sobre a main, reordenando o patch acordado para o começo, e depois fazer fast-forward da main para esse patch
    Essencialmente, a ideia é reduzir o escopo da revisão para “as partes ainda em discussão”, enquanto as correções de bugs são mescladas assim que ficam prontas
    A objeção seria “basta criar uma revisão/PR separado”, mas aí surge a complexidade de o patchset A depender do patchset B e, depois que B é mesclado, passar a depender da main

    • Continuo repetindo, mas o Gerrit faz isso basicamente por padrão :) No Gerrit, você consegue ver a relação entre quaisquer dois patches e, mais importante, ele mostra cada patch individualmente. Então, em uma série A -> B -> C, se B for pequeno e estiver bom, dá para colocá-lo imediatamente
      Ideias inteligentes de UX como o Attention Set também ajudam nisso. Basicamente, ele indica “quem é a pessoa que precisa tomar a próxima ação?” e funciona como um jogo por turnos. Se acabei de revisar, não estou mais no attention set daquele patch; o autor está
      Por isso, na UI do Gerrit, esse item desce na minha fila. No topo da fila ficam os itens em que eu estou no attention set. Naturalmente, o trabalho se agrupa dessa forma
      Nem entrei em outros pequenos problemas irritantes da UX do GitHub, mas até a lista de pull requests é pior do que as alternativas. Não dá para saber em que estado cada item está, então você acaba tendo que ler tudo
    • O Gerrit consegue fazer exatamente isso. Se você fizer push de uma cadeia x-b-c-d-e, ela aparece empilhada na UI, mas dá para fazer cherry-pick de b para a main facilmente, verificar se o CI passou e se a revisão normal está ok, e depois fazer rebase do restante por cima disso
      Se for x, que está na base, dá para submetê-lo diretamente e continuar com o restante
  • É sempre interessante ver uma nova abordagem para code review. O GitHub também tem suas vantagens, mas está longe de ser perfeito
    No cenário apresentado, dá para considerar dividir os três patches em pull requests separados que dependem uns dos outros. O GitHub não oferece suporte nativo a isso, mas uma ferramenta adequada de code review deveria conseguir manter os pull requests pequenos e, ao mesmo tempo, gerenciar dependências. Por exemplo, o patch 3 pode depender do patch 2, e o patch 2 do patch 1
    Graças ao rastreamento de dependências fornecido pela ferramenta, dá para garantir que todos sejam implantados juntos, se necessário
    Ao revisar cada patch separadamente, o feedback fica mais claro e é mais fácil responder a ele. Também dá para fazer squash dos commits dentro do pull request e criar um histórico de commits limpo que reflita com precisão cada alteração individual. Além disso, com a ferramenta adequada, dá para usar IA para resumir pull requests e reviews, simplificando também a criação de mensagens de commit precisas sem trabalho manual
    Uma boa ferramenta de code review não deveria ficar à mercê de operações de git como rebase, merge e force push. Por mais complexas que sejam as operações de git acontecendo por trás, o revisor deveria ver apenas as mudanças desde a última review. Assim, ele pode se concentrar no conteúdo novo sem revisar novamente um diff grande, e o histórico de review permanece limpo e separado do histórico de commits
    Fico curioso se essa forma de dividir pull requests e rastrear interdependências atenderia aos requisitos

    • A expressão “nova abordagem para code review” é positiva e respeitosa até com os incumbentes, mas isso não é uma ideia nova de forma alguma
      É literalmente o fluxo de trabalho para o qual o git foi projetado para dar suporte, e é o cerne de uma crítica antiga que existe desde que o GitHub passou a ter pull requests
      Fico curioso por que essa ideia, deixa eu olhar o calendário, foi depreciada por mais de 15 anos como a forma de trabalhar dos “dinossauros das mailing lists” e só agora está chegando aos círculos de desenvolvimento em voga
      Eu achava que muitos desenvolvedores de software se recusavam havia muito tempo a aprender direito uma das ferramentas que mais usam e, por isso, estavam escolhendo deliberadamente fluxos de trabalho ruins. Foi por isso que escolhi e criei esse tipo de ferramenta e fluxo quando migrei a empresa para git. Afinal, ninguém é demitido por comprar IBM
    • Pelo que sei, dividir uma série em PRs individuais só funciona direito se você tiver permissão de commit no repositório. Assim dá para colocar um PR em cima de outro branch do repositório principal, e não em cima da main
      Quando um contribuidor externo usa um fork do repositório, os três PRs acabam contendo progressivamente A, A+B e A+B+C. Aí, nos dois últimos PRs, você precisa ver de novo no diff até código que já foi revisado em outro PR, o que dificulta a review
    • O autor já tem um branch e seus commits. Então qual é o objetivo de dividir o branch em três pull requests, cada um contendo um commit?
      Cada commit pode ser revisado independentemente. Também dá para rastrear dependências entre commits. O fato de não haver IA embutida também é um ponto positivo. Ao contrário dos PRs, também dá para ver o interdiff entre commits
      Commits são o material básico do Git. É só usar commits
    • Como já foi mencionado em outro ponto desta thread, essa também é a abordagem seguida pelo Sapling
      Fico curioso sobre como o GitContext rastreia commits que passaram por operações como fixup, rebase e reordenação
    • Não daria simplesmente para usar o velho esquema de merge train? O pull request A aponta para o branch B, B aponta para master, depois você faz merge de B em master e, em seguida, aponta A de volta para master; não é isso, ou estou perdendo o ponto?
  • Sou a pessoa que escreveu este texto. Perguntem o que quiserem
    Além disso, não sei se isso viola as regras, mas recentemente passei a precisar de um novo emprego. Gosto de trabalhar com ferramentas de desenvolvimento e problemas difíceis. Se você gostou deste texto, quer aumentar a produtividade da sua equipe de desenvolvimento ou quer experimentar e aproveitar meu excelente e às vezes eclético gosto, meu e-mail está no perfil

    • Obrigado pela boa risada :)
      Falando sério, seu currículo é impressionante. Espero que você encontre algo bom, e rápido. Pela minha experiência bem recente procurando emprego, o mercado de contratação atual está mortalmente doente. Quanto mais sênior e experiente você é, mais as práticas insanas de entrevistas/RH e as rejeições inexplicáveis machucam a alma
      Um amigo me enviou os links abaixo
      https://danluu.com/hiring-lemons/
      https://danluu.com/programmer-moneyball/
      https://danluu.com/algorithms-interviews/
      Boa sorte
    • Você resumiu muito bem o modelo mental do estilo de review por interdiff e os problemas da abordagem de code review do GitHub
      Uma coisa que ainda não foi abordada é que, por causa do diff soup, as pessoas podem passar a preferir a estratégia de squash merge para se livrar do “ruído” dos commits de fixup. Com isso, acabam jogando fora também os 3 bons commits atômicos iniciais
      No estilo de review por interdiff, os 3 commits iniciais permanecem, e a decisão de aplicá-los individualmente ou fazer squash pode ser baseada inteiramente no próprio commit e em sua atomicidade real
    • Pela minha experiência usando a parte gratuita do GitHub, a expressão diff soup é muito precisa. A versão paga é melhor? O GitLab consegue ficar mais próximo do Gerrit?
      Também existem serviços externos que tentam reduzir a dor do GitHub e oferecem suporte a stacked diff, mas parecem bem caros. Especialmente em comparação com um Gerrit self-hosted. Fico curioso se você também avaliou essas ferramentas
    • Fico curioso sobre o que você acha do GitLab configurado com Merge Request Dependencies e a estratégia de merge Squash+Merge
      Lembro que, no passado, eu conseguia configurar um padrão de série de MRs com bastante facilidade. Ao fazer merge de 3 MRs, eram criados 3 merge commits, cada um com um único commit squash por MR. Isso era independente do histórico de commits do MR
      O trade-off é que, se um branch mais no início da série mudar durante a review, é preciso fazer merge novamente nos branches posteriores da série
      Ainda assim, durante a review, as pessoas podem mexer no histórico de commits como quiserem. Como tudo será squashado de qualquer forma, não importa
      Mas faz tempo que fiz isso e, hoje em dia, como estou preso ao GitHub, talvez eu esteja vendo isso por lembranças cor-de-rosa
  • O Review Board [https://www.reviewboard.org] já tinha criado o interdiff em 2006. Na verdade, talvez eu tenha cunhado esse termo, ou chegado a ele de forma independente.
    Ainda hoje é minha parte favorita do produto e do processo de revisão de código, e é um dos recursos que mais ouço as pessoas dizerem que sentem falta quando migram para algo como o GitHub.
    Commits de fix-it nunca me pareceram uma alternativa adequada.
    Primeiro, eles não mostram como a alteração superior foi incorporada à série de commits.
    Segundo, mesmo que temporariamente, bagunçam o grafo de commits e dificultam a revisão. Quem já vinha acompanhando a revisão pode já ter lido o código que o commit de fix-it corrige, mas alguém que está chegando agora pode começar com uma má primeira impressão do que aquele código pretende fazer ou de como está estruturado.
    Terceiro, nem todo mundo usa Git ou um sistema de controle de versão capaz de lidar com vários commits. Em desenvolvimento de jogos, é comum usar Perforce; em fabricantes de chips, muitas vezes se usam sistemas de controle de versão especializados, como o Keysight SOS. Nesses lugares, commits de fix-it simplesmente não são uma opção.
    Com um sistema de revisão de código com suporte adequado a interdiff, um revisor pode acompanhar todas as atualizações desde a primeira solicitação de revisão publicada e ver apenas o que mudou, enquanto outro revisor pode entrar direto na alteração completa mais recente sem se preocupar com uma série de fix-its. E isso é possível independentemente do sistema de controle de versão.
    Se for bem feito, também pode coexistir com alterações compostas por vários commits.
    Por exemplo, você pode dividir um projeto pequeno em vários commits — handlers de API, UI de frontend e documentação — para ajudar na revisão, e decidir que, como os commits são fortemente relacionados, faz sentido publicá-los como uma única solicitação de revisão. Se forem pouco relacionados entre si, o ideal provavelmente seria criar várias solicitações de revisão em uma cadeia de dependências.
    Conforme o feedback da revisão, você pode criar uma série de mudanças em um ou em todos esses commits. Quando as pessoas revisarem a atualização, seria bom que pudessem ver como cada parte evoluiu, em vez de mapear mentalmente o commit de fix-it e sua alteração para a mudança original correspondente.
    Por isso o interdiff é realmente excelente. Seja com muitos commits pequenos ou um commit grande, seja uma solicitação de revisão de commit único ou de múltiplos commits, mais pessoas deveriam usá-lo.

    • Realmente fantástico. Na verdade, na parte 2 deste texto pretendo tratar da história e do contexto político que nos trouxeram até essas ferramentas.
      Por volta de 2013, o Glasgow Haskell Compiler migrou do método que, em tom de brincadeira, chamávamos de “ler arquivos .patch no bug tracker” para o uso do Phabricator. Havia alguns motivos, mas na época não era por causa de stacked diff. Era porque o GitHub era tão ruim para revisão que nem side-by-side diff tinha. Só por esse motivo ele já estava completamente fora de cogitação, e havia outros também.
      Mas aquela não foi a primeira vez que levei uma equipe para uma ferramenta de revisão de código. Meu primeiro emprego, em 2009, era em uma equipe de engenharia muito pequena e unida, reunida em uma sala em Houston, e eu achava que, se outras pessoas revisassem meu código e eu lesse o que elas escreviam, eu conseguiria entender melhor a base de código.
      Então, nos primeiros meses depois de entrar, insisti com meu gerente até ele instalar o ReviewBoard, e todo mundo gostou muito.
      Então fica meu agradecimento ao RB. Ainda me lembro dele com carinho de vez em quando. Graças a isso, revisão de código foi uma parte enorme da minha carreira praticamente desde o primeiro dia, e ainda acho que poderíamos fazer mais disso.
  • No geral, senti que sistemas como o Gerrit, centrados em revisão de código primeiro e baseados em rebase, tornam o código muito mais fácil de revisar.
    Uma das melhores partes é o suporte nativo a empilhar vários patches, o que leva as pessoas a criar patches menores e mais fáceis de revisar.
    A revisão de código do GitHub parece um recurso ruim acrescentado depois: uma UI que desperdiça espaço e parece uma thread de fórum, incapacidade de acompanhar além de um rebase, e assim por diante.

    • Há uma coisa de que sinto falta ao fazer push de uma pilha de commits no Gerrit: não existe um lugar central para falar sobre a pilha inteira, em vez de commits individuais.
      Esse tipo de discussão técnica, mas de “visão geral”, acaba acontecendo com frequência demais no issue tracker. Mas também não sei onde ela deveria ficar. Essa pilha é temporária demais e pode mudar completamente no próximo push.
  • Já ouvi algumas vezes gente defendendo essa estratégia, mas não me convenci. A maioria dos projetos em que trabalho faz squash da feature branch em um único commit ao mesclar, e apaga o histórico da branch
    Se eu estivesse no caso descrito pelo autor, simplesmente faria refatoração, nova API e atualização como 3 commits
    O que foi claramente uma boa prática para mim foi evitar branches de longa duração. Esses cenários de commits em múltiplas etapas geralmente surgem daí. Aquela situação em que alguém trabalha em uma funcionalidade por dias, às vezes semanas, e depois quer jogar tudo de uma vez
    Prefiro muito mais levar as mudanças para a main diariamente, ou até várias vezes por dia. Uma forma de fazer isso é usar feature flags para que commits em andamento também possam ser integrados sem problemas. Se o sistema estiver bem configurado, há também a vantagem de poder ativar em ambientes de desenvolvimento e staging para testar. Fica a um passo de deploy blue/green
    Não quero uma forma de revisar commits grandes com mais facilidade. Quero forçar a equipe a commitar mudanças pequenas cedo e com frequência. Entendo que nem todo mundo concorde

    • Quem prefere stacked diff diz que fica mais fácil integrar mudanças mais rapidamente, independentemente do tamanho
      Um dos motivos é que, ao contrário dos PRs do GitHub, é possível integrar apenas parte da stack. No exemplo do texto, se o diff de “pequena refatoração” estiver pronto, dá para colocá-lo primeiro sem integrar junto os diffs de “nova API” e “migração dos usuários da API”
      Ao manter o foco em commits, não em branches, o efeito é ter commits menores no total do que com branches de longa duração
    • Fazer squash de uma feature branch em um único commit e apagar o histórico da branch é horrível
      Isso torna git blame e git bisect praticamente inúteis
      Se houver uma regressão, git bisect pode reduzir o problema a um único patch. Por isso, para uma funcionalidade específica, é melhor ter no histórico do git 50 patches de 160 linhas do que um único patch de 8000 linhas
      Se uma linha de código parecer suspeita, quero que git blame, ou uma sequência de comandos git blame, me leve a um commit de 160 linhas com uma mensagem detalhada, não a um commit de 8000 linhas
      A ordem dos commits originais também deve ser preservada. Anos depois, só ler as mensagens individuais dos commits em ordem pode ajudar muito a entender o projeto original. Claro que o patchset original precisa estar organizado em ordem de dependência, compilar em cada etapa etc., o que exige uma etapa de desenvolvimento separada sobre a implementação da funcionalidade. O código deve ser apresentado em etapas lógicas
  • Interessante. No trabalho, usamos PRs da forma como o autor usa commits e, no fim, de fato fazemos squash-and-merge
    Mas esse método exige rebase dos PRs posteriores sempre que um PR anterior muda. É bem trabalhoso, esbarra na regra de “não fazer force-push”, leva tempo para os engenheiros aprenderem e tende a quebrar comentários existentes de revisão de código na UI do GitHub. Ainda assim, funciona razoavelmente com dois ou três PRs
    No nosso fluxo, commits são mais pontos de salvamento do que unidades de trabalho
    Também usamos bastante stash e o undo-tree-mode do Emacs. Assim, há quatro formas de rastrear o histórico do código-fonte, o que soa redundante, mas na prática funciona bem
    A usabilidade do Git para lidar com isso é bem ruim. O Phabricator era melhor, mas ainda assim desnecessariamente difícil. Talvez uma nova ferramenta de controle de versão possa oferecer, como conceitos de primeira classe, abstrações de nível mais alto que commits e branches; ou talvez isso a torne ainda pior de usar

    • Vocês estão usando commits como PRs. Por isso as dependências entre PRs complicam as coisas
      Seria bom ter uma ferramenta melhor que o Git? Claro, eu também quero uma ferramenta com menos falhas e mais amigável. Mas, como paliativo, que tal tratar commits como commits? Sem se complicar de propósito
      Fico me perguntando qual é o tamanho da responsabilidade do GitHub por fazer as pessoas enxergarem tudo pelas lentes dos PRs
    • Vale dar uma olhada em jj, sapling e mercurial
  • Gostei do post do blog, e é bom ver pessoas realmente pensando em como revisão de código deveria funcionar
    Usei bastante quatro sistemas de revisão de código com prós e contras diferentes: Critique, interno do Google; Gerrit, que usei no Google mas é igual ao externo; GitHub; e o CodeApprove, que eu criei
    O Critique era disparado o melhor, mas isso era possível porque se encaixava perfeitamente no monorepo do Google, no sistema próprio de controle de versão e nas ferramentas customizadas de lint/test. O CodeApprove foi projetado para trazer o máximo possível dessas vantagens para o GitHub, mas, na prática, é difícil chegar perto
    O Gerrit era o segundo melhor em termos de experiência do revisor, mas eu sempre detestei como autor. Parecia haver mais formas de fazer errado do que de fazer certo, e a UI também não é particularmente bonita
    O GitHub é muito amigável para o autor e funciona do jeito que pensamos: escrevemos código, recebemos feedback, escrevemos mais código. Se fizer squash and merge no fim do PR, não há o problema de histórico mencionado pelo autor
    Mas ele não é muito amigável para revisores ou equipes. Diffs incrementais não são destacados, o diff e a conversa ficam em abas diferentes, e force push e rebase destroem o histórico. Comentários somem como “outdated”, não dá para comentar em arquivos fora da janela de diff, arquivos grandes são ocultados por padrão, e por aí vai. É evidente que o GitHub não se importa muito com isso, e talvez eles saibam de algo que eu não sei
    No fim, o mais frustrante é que muitas equipes simplesmente aceitam a ferramenta de revisão de código embutida na plataforma de controle de versão. É como usar o IDE que veio pré-instalado no notebook. Hoje há opções muito melhores. Além do CodeApprove, gosto do GitContext, Reviewable e Graphite, e poderia citar pelo menos mais meia dúzia de ótimas opções. Não deveríamos aceitar o padrão

    • O problema é o GitHub não ser amigável para revisores ou equipes
      Revisores e mantenedores são muito mais escassos que contribuidores. Acho que o fluxo de trabalho e a UI deveriam otimizar a vazão de revisões
      Não usei as outras três ferramentas mencionadas, então meu argumento é geral