Como as alterações não funcionais devem ser confirmadas?

8

Estou trabalhando em uma base de código herdada de pequeno a médio porte e, ao trabalhar em um ticket, encontrarei o código que deve ser limpo ou que preciso fazer a limpeza apenas para poder entender o acompanhamento do aplicativo.

Um exemplo real é:

if( !a && b ){
     doSomething1();
     doSomething2();
     doSomething3();
     doSomething4();
     doSomething5();
}else if( a ){
     doSomething1();
     doSomething2();
     doSomething3();
     doSomething4();
     doSomething5();
}else if( !b && ( a || c )  ){
     doSomething1();
     doSomething2();
     doSomething3();
     doSomething4();
     doSomething5();
}

Outro é corrigir erros de digitação e Engrish em comentários e documentação em uma dúzia de arquivos de origem.

No entanto, muitas vezes essa limpeza acaba não relacionada ao problema principal e eu me pergunto como é melhor fazer a limpeza. Do meu ponto de vista, existem três opções:

  1. Antes da correção: isso funciona cronologicamente, pois é a ordem em que ocorrem, mas se eles quebram algo, complicam a correção e torna mais difícil diferenciá-la com o que estava em produção. Ele também introduz um commit adicional que é ruído.
  2. Com a correção: mas isso obscurece a substituição real do código defeituoso por arquivos nos quais findGeoragphyestava correto findGeography.
  3. Após a correção: isso requer remoção e limpeza feitas, que ajudaram você a entender o código e, em seguida, testá-la novamente, confirmar a correção e, em seguida, voltar e refazer a limpeza. Isso permite a comparação mais limpa do código defeituoso, mas duplica o esforço e pode levar a confirmações falsas também.

tl; dr: Então, qual é o melhor método para confirmar o código de limpeza?

Contexto: Não temos testes de unidade e produtos de desenvolvimento executando alterações, dando-lhes uma olhada e jogando-os por cima do muro para o controle de qualidade para validar através de correções de regressão manual. Além disso, se não fizermos algumas limpezas de formulários, o código se tornará ininteligível. Sei que isso está longe de ser o ideal, mas esse é um verdadeiro empreendimento empresarial que coloca comida na minha mesa e não é minha escolha.

version-control Trenó
fonte
Sem testes de unidade? Por que não escrever antes da correção? (se apropriado)
Wolf
O código não era testável por unidade, porque era altamente dependente do banco de dados e não havia especificação do que deveria fazer; portanto, os testes de unidade teriam sido "testStillDoesWhatItDoes ()" em vez de algo comportamental ou semântico e com violações de camadas, teria sido realmente difícil de zombar também. Além disso, muitas mudanças ocorreram nas oportunidades, como a correção dos nomes dos métodos do Engrish, como eu os conheci, mas não tiveram tempo para escrever testes e eles permaneceriam no Engrish de outra forma.
Trenó
veja também: Vale a pena fazer um commit apenas para resolver erros não críticos?
mosquito

Respostas:

11

Sigo um processo semelhante à resposta de Karl. Prefiro uma confirmação separada da limpeza / refatoração antes das alterações do recurso por alguns motivos:

  • A separação dos dois commits explicará seu processo e o pensamento melhor para outras pessoas que analisam os logs.
  • Ele fornece um conjunto específico de alterações úteis durante uma revisão de código.
  • Isso significa que, se você deseja reverter as alterações de recursos, ainda mantém a limpeza / refatoração.
  • As alterações de limpeza podem ser testadas separadamente e antes das atualizações de recursos.
Matt S
fonte
6

Eu prefiro o commit extra antes da correção. Isso separa claramente as duas tarefas que você está executando e permite que outras pessoas vejam o que foi a limpeza e o que foi uma correção de bug. Além disso, a limpeza (pelo menos para mim) geralmente é muito mais rápida; portanto, se eu fizer uma correção de limpeza, não preciso me preocupar tanto com alguém fazendo uma alteração difícil de mesclar enquanto estiver trabalhando por mais tempo. tarefa.

Eu tenho usado o "comprometer com a correção" abordagem em locais com as políticas que requerem uma ID de solicitação de mudança com todos os cometem, e não permitem aos desenvolvedores criar novas solicitações de mudança apenas para o código de limpeza. Sim, essas políticas são improdutivas, mas existem locais de trabalho como esse.

Karl Bielefeldt
fonte
5
Em relação ao segundo parágrafo: Em tais situações, às vezes reutilizo o ID do ticket da correção que estou fazendo, mas ainda coloco a limpeza em um commit separado.
Bart van Ingen Schenau
0

Se a correção precisar ser corrigida em uma versão existente, você deve fazer o check-in primeiro e depois a limpeza depois disso, ou você (pelo menos parte do tempo) está criando a pessoa que precisa aplicar a correção trabalhar mais do que o necessário (para entender o que mudou ou para corrigir as mudanças cosméticas antes de corrigir a correção real (observarei que fiz mais do que algumas mudanças cosméticas declaradas que acabaram quebrando alguma coisa, portanto, corrigindo mais do que a alteração mínima para corrigir um defeito pode aumentar o risco de liberar algo ruim).

Sim, isso é trabalho extra. Sim, é uma dor. Mas sim, é o caminho certo para fazer as coisas. Meu fluxo de trabalho normal é fazer tudo em uma caixa de proteção (limpeza e correção de defeitos), criar uma nova caixa de proteção, aplicar apenas uma alteração mínima na correção de defeitos, verificar essa alteração na caixa de proteção original e verificar a limpeza- código up. Na minha experiência, não é muito trabalho extra.

James McLeod
fonte