É uma prática ruim não excluir arquivos redundantes imediatamente do VCS, mas marcá-los como “Para ser excluído” com comentários primeiro?

53

Eu queria saber se a maneira como lidei com os arquivos de origem que precisam ser excluídos do controle de versão pode ser considerada uma prática ruim.

Eu quero explicar para você com base nesse exemplo:

Recentemente, fiquei muito irritado porque tive que classificar de forma tediosa as classes Java em um programa que era basicamente código morto, mas não foi documentado em nenhum lugar e também não foi comentado nessas classes Java. É claro que eles precisavam ser deletados, mas antes que eu exclua essas coisas redundantes eu tenho um - alguns podem dizer estranho - hábito:

Eu não excluo esses arquivos redundantes imediatamente via SVN- > Delete (substitua pelo comando delete do sistema de controle de versão de sua escolha), mas coloque comentários nesses arquivos (eu me refiro tanto na cabeça quanto no rodapé) eles serão deletados + meu nome + a data e também - mais importante - POR QUE ELES SÃO APAGADOS (no meu caso, porque eles estavam mortos, código confuso). Em seguida, salve e confirme para o controle de versão. Da próxima vez que eu tiver que confirmar / verificar algo no projeto para controle de versão, eu pressiono SVN- > Delete e eles são eventualmente deletados no Controle de Versão - ainda é claro restaurável através de revisões e é por isso que eu adotei esse hábito .

Por que fazer isso em vez de excluí-los imediatamente?

Meu motivo é que eu quero ter marcadores explícitos pelo menos na última revisão em que esses arquivos redundantes existiam, por que eles mereciam ser excluídos. Se eu os excluir imediatamente, eles serão excluídos, mas não serão documentados por que foram excluídos. Eu quero evitar um cenário típico como este:

"Hmm... why were those files deleted? I did work fine before." (Presses 'revert' -> guy who reverted then is gone forever or not available in the next weeks and the next assignee has to find out tediously like me what those files are about)

Mas você não percebe porque esses arquivos foram deletados nas mensagens de commit?

Claro que sim, mas uma mensagem de commit às vezes não é lida pelos colegas. Não é uma situação típica que, quando você tenta entender o código (no meu caso, morto), verifique primeiro o log de controle de Versão com todas as mensagens de confirmação associadas. Em vez de rastrear o log, um colega pode ver imediatamente que esse arquivo é inútil. Ele economiza seu tempo e ele / ela sabe que esse arquivo provavelmente foi restaurado para o mal (ou pelo menos levanta uma questão.

    
por Bruder Lustig 14.06.2017 / 18:01
fonte

7 respostas

110

O problema de adicionar um comentário a um arquivo que deve ser excluído, em vez de excluí-lo no controle de origem e colocar a explicação nele, pressupõe que, se os desenvolvedores não lerem mensagens de confirmação, eles certamente lerão comentários na origem código.

Do ponto de vista de um estranho, essa metodologia parece estar enraizada em uma visão muito conservadora do controle de origem.

"E se eu excluir esse arquivo não utilizado e alguém precisar dele?" alguém pode perguntar.

Você está usando o controle de origem. Reverta a mudança ou, melhor ainda, fale com a pessoa que apagou o arquivo (comunicar).

"E se eu deletar o arquivo morto, então alguém começa a usá-lo novamente e eles fazem alterações?" alguém pode perguntar.

Novamente, você está usando o controle de origem. Você terá um conflito de mesclagem que uma pessoa deve resolver. A resposta aqui, como na última pergunta, é se comunicar com seus colegas de equipe.

Se realmente duvida que um arquivo deve ser removido, comunique-o antes excluindo-o do controle de origem. Talvez ele tenha parado de ser usado recentemente, mas um recurso futuro pode exigir isso. Você não sabe disso, mas um dos outros desenvolvedores pode.

Se precisar ser removido, remova-o. Cortar a gordura da base de código.

Se você fez um "oopsie" e realmente precisa do arquivo, lembre-se de que está usando o controle de origem para recuperar o arquivo.

Vincent Savard, em um comentário sobre a questão, disse:

... If your colleagues don't read the commit messages and they resurrect a file that was rightfully deleted and it passes code reviewing, there's definitely something wrong in your team, and it's a great opportunity to teach them better.

Este é um bom conselho. Revisões de código deveriam estar pegando esse tipo de coisa. Os desenvolvedores precisam estar consultando mensagens de confirmação quando uma alteração inesperada é feita em um arquivo ou um arquivo é removido ou renomeado.

Se as mensagens de confirmação não contar a história, os desenvolvedores também precisarão escrever melhores mensagens de confirmação.

Ter medo de excluir códigos ou excluir arquivos é indicativo de um problema sistêmico mais profundo com o processo:

  • Falta de comentários precisos sobre o código
  • Falta de compreensão sobre como o controle de origem funciona
  • Falta de comunicação da equipe
  • Mensagens de comprometimento insuficientes por parte dos desenvolvedores

Esses são os problemas a serem resolvidos, então você não sente que está jogando pedras em uma estufa quando apaga código ou arquivos.

    
por 14.06.2017 / 18:54
fonte
105

Sim, é uma prática ruim.

Você deve colocar a explicação para a exclusão na mensagem de confirmação quando você confirmar a exclusão dos arquivos.

Os comentários nos arquivos fonte devem explicar o código como parece atualmente . Mensagens de commit devem explicar porque as alterações no commit foram feitas, então o histórico de commit no arquivo explica seu histórico.

Escrever comentários explicando as alterações diretamente na própria fonte apenas tornará o código muito mais difícil de seguir. Pense nisso: se você comentar no arquivo toda vez que você alterar ou excluir o código, em breve os arquivos serão inundados com comentários sobre o histórico de alterações.

    
por 14.06.2017 / 18:06
fonte
15

Na minha opinião, ambas as suas opções são não são as melhores práticas , ao contrário de más , prática:

  1. Adicionar comentários apenas adiciona qualquer valor se alguém ler esses comentários.
  2. A simples exclusão do VCS (com um motivo na descrição da alteração) pode afetar um produto essencial e muitas pessoas não leem a descrição da alteração, especialmente quando estão sob pressão.

Minha prática favorita - em grande parte devido a ter sido mordida várias vezes ao longo dos anos, tendo em mente que você nem sempre sabe onde ou por quem seu código está sendo usado - é:

  1. Adicione um comentário informando que é um candidato para exclusão e uma depreciação #warning ou semelhante, (a maioria dos idiomas possui algum tipo de mecanismo, por exemplo, em Java , no pior dos casos, uma declaração print ou similar será suficiente, idealmente com algum tipo de escala de tempo e com detalhes de contato. Isso alertará qualquer um que ainda esteja realmente usando o código. Estes avisos estão normalmente dentro de cada função ou classe se tais coisas existem .
  2. Após algum tempo, atualize o aviso para um escopo de arquivo padrão #warning (algumas pessoas ignoram os avisos de descontinuação e algumas cadeias de ferramentas não exibem esses avisos por padrão).
  3. Posteriormente, substitua o escopo do arquivo #warning por #error ou equivalente - isso quebrará o código no momento da criação, mas poderá, se for absolutamente necessário, ser removido, mas a pessoa que o remover não poderá ignorá-lo. (Alguns desenvolvedores não lêem ou endereçam nenhum aviso ou têm tantos que não podem separar os mais importantes).
  4. Por fim, marque o (s) arquivo (s), (em ou após a data de vencimento), como excluído no VCS.
  5. Se você excluir um pacote / biblioteca / etc inteiro no estágio de aviso, eu adicionaria um README.txt ou README.html ou similar com as informações sobre quando & Por que é planejado para se livrar do pacote e deixar apenas esse arquivo, com uma mudança para dizer quando foi excluído, como o único conteúdo do pacote por algum tempo depois de remover o pacote resto do conteúdo.

Uma vantagem dessa abordagem é que alguns sistemas de versionamento (CVS, PVCS, etc.) não removerão um arquivo existente no checkout se ele tiver sido excluído no VCS. Também fornece desenvolvedores conscientes, aqueles que corrigem todos os seus avisos, muito tempo para resolver o problema ou apelar da exclusão. Isso também força os desenvolvedores remanescentes a, pelo menos, olhar o aviso de exclusão e reclamar muito .

Observe que #warning / #error é um mecanismo C / C ++ que causa um aviso / erro seguido pelo texto fornecido quando o compilador encontra esse código, java / java-doc possui @Deprecated de anotação para emitir um aviso sobre o uso & @deprecated para fornecer algumas informações, etc. há receitas fazer semelhante em python & Eu vi assert usado para fornecer informações semelhantes para #error no mundo real.

    
por 14.06.2017 / 20:46
fonte
3

Sim, eu diria que é uma prática ruim, mas não porque esteja nos arquivos versus na mensagem de confirmação. O problema é que você está tentando fazer uma alteração sem se comunicar com sua equipe.

Sempre que você fizer qualquer alteração no tronco - adicionando, excluindo ou modificando arquivos de qualquer forma - você deve, antes de se comprometer com o tronco, falar sobre essas mudanças diretamente com um membro ( ou membros) da equipe que estaria diretamente familiarizada com eles (essencialmente, discuta o que você colocaria diretamente nesses cabeçalhos com seus colegas de equipe). Isso garantirá que (1) o código que você está excluindo realmente precisa ser excluído e que (2) sua equipe terá mais chances de saber que o código foi excluído e, portanto, não tentará reverter suas exclusões. Sem mencionar que você também terá a detecção de bugs e coisas assim para o código que você adicionar.

É claro que, quando você modifica coisas grandes, você também deve colocá-las na mensagem de commit, mas como você já discutiu, não precisa ser a fonte de anúncios importantes. A resposta de Steve Barnes também aborda algumas boas estratégias a serem usadas se o grupo potencial de usuários para o seu código for muito grande (por exemplo, usando os marcadores preteridos da sua linguagem em vez de excluir os arquivos no início).

Se você não quer passar tanto tempo sem cometer (ou seja, sua mudança faz mais sentido com várias revisões), é uma boa idéia criar uma ramificação a partir do tronco e confirmar na ramificação, depois mesclar a ramificação novamente tronco quando a mudança está pronta. Dessa forma, o VCS ainda está fazendo backup do arquivo para você, mas suas alterações não estão afetando o tronco de forma alguma.

    
por 15.06.2017 / 09:20
fonte
1

Um problema relacionado de projetos que se baseiam em várias plataformas e configurações. Só porque eu determino que está morto não significa que seja uma determinação correta. Note que a morte em uso ainda pode significar dependência vestigial que ainda precisa ser eliminada, e algumas podem ter sido perdidas.

Então (em um projeto C ++) eu adicionei

#error this is not as dead as I thought it was

E verifiquei isso. Espere que ele passe pela reconstrução de todas as plataformas normais e que ninguém se queixe disso. Se alguém o encontrasse, seria fácil remover uma linha em vez de ficar perplexa com um arquivo ausente.

Concordo em princípio que os comentários que você mencionou estão duplicando o recurso que deve ser feito no sistema de gerenciamento de versões . Mas você pode ter razões específicas para suprir isso. Não saber se a ferramenta VCS não é uma delas.

    
por 16.06.2017 / 20:59
fonte
-1

No continuum de Bad Practices, isso é bem menor. Eu vou fazer isso de vez em quando, quando eu quiser dar uma olhada em um branch e poder ver o código antigo. Isso pode facilitar a comparação com o código de substituição. Eu normalmente recebo um comentário de revisão de código "cruft". Com uma pequena explicação eu passo a revisão de código.

Mas esse código não deve durar muito tempo. Eu geralmente apago no começo do próximo sprint.

Se você tiver colegas de trabalho que não leem mensagens de confirmação ou não perguntarão por que você deixou o código no projeto, seu problema não é de estilo de codificação incorreto. Você tem um problema diferente.

    
por 18.06.2017 / 14:36
fonte
-3

você também pode refatorar a classe e renomeá-la com um prefixo para UNUSED_Classname antes de extrair seu truque. Então você deve cometer diretamente a operação de exclusão também

  • Etapa 1: renomeie a classe, adicione seu comentário, confirme
  • Etapa 2: excluir arquivo e confirmar

Se for código morto, exclua-o. Alguém precisa disso, eles podem voltar, mas a renomeação do passo irá impedi-los de usá-lo diretamente, sem pensar.

Também ensine as pessoas a ver todas as mensagens de commit de um arquivo, algumas pessoas nem sabem que isso é possível.

    
por 15.06.2017 / 05:34
fonte