Os comentários inline 'editados por' são a norma em lojas que usam controle de revisão?

17

O desenvolvedor sênior em nossa loja insiste em que, sempre que o código for modificado, o programador responsável deve adicionar um comentário embutido informando o que ele fez. Esses comentários geralmente se parecem// YYYY-MM-DD <User ID> Added this IF block per bug 1234.

Usamos o TFS para controle de revisão, e parece-me que comentários desse tipo são muito mais apropriados como notas de check-in do que ruído embutido. O TFS ainda permite associar um check-in a um ou mais bugs. Alguns de nossos arquivos de classe mais antigos, frequentemente modificados, parecem ter uma proporção de comentário / LOC aproximando-se de 1: 1. A meu ver, esses comentários tornam o código mais difícil de ler e agregam valor zero.

Essa é uma prática padrão (ou pelo menos comum) em outras lojas?

Joshua Smith
fonte
12
Eu concordo com você, é para isso que servem os logs de revisão no controle de versão.
TZHX
1
O desenvolvedor sênior da sua equipe fez isso antes da disseminação do controle de versão e não chegou à conclusão de que ele pertence a outro lugar para remover o cheiro do código. Mostre a ele seis projetos de código aberto que usam o sistema bugzilla e vc, pergunte se ele precisava saber nos comentários da biblioteca do jQuery que o $ .ajax () foi recentemente alterado por jaubourg há 4 meses e todas as pequenas alterações feito por centenas de pessoas pertencem a lá também. Toda a biblioteca jQuery seria uma bagunça de comentários e nada foi ganho!
Incognito
1
Na verdade, temos uma política não escrita de nenhum nome no código-fonte, pois ele pode criar um senso de propriedade do código, que é considerado um BadThing TM.
precisa saber é o seguinte

Respostas:

23

Geralmente considero esses comentários uma prática ruim e acho que esse tipo de informação pertence aos logs de confirmação do SCM. Isso apenas torna o código mais difícil de ler na maioria dos casos.

No entanto , muitas vezes ainda faço algo assim para tipos específicos de edições.

Caso 1 - Tarefas

Se você usa um IDE como Eclipse, Netbeans, Visual Studio (ou tem alguma maneira de fazer pesquisas de texto em sua base de código com qualquer outra coisa), talvez sua equipe use algumas "tags de comentários" ou "tags de tarefas" específicas. Nesse caso, isso pode ser útil.

De vez em quando, ao revisar o código, adicionava algo como o seguinte:

// TOREVIEW: [2010-12-09 haylem] marking this for review because blablabla

ou:

// FIXME: [2010-12-09 haylem] marking this for review because blablabla

Eu uso diferentes tags de tarefas personalizadas que posso ver no Eclipse na visualização de tarefas para isso, porque ter algo nos logs de confirmação é uma coisa boa, mas não é suficiente quando você tem um executivo perguntando em uma reunião de revisão por que a correção XY foi completamente esquecida e escorregou. Portanto, em assuntos urgentes ou trechos de código realmente questionáveis, isso serve como um lembrete adicional (mas geralmente manterei o comentário curto e verificarei os logs de confirmação, porque É para isso que serve o lembrete, para que eu também não confunda o código Muito de).

Caso 2 - Patches de libs de terceiros

Se meu produto precisar empacotar um código de terceiros como fonte (ou biblioteca, mas reconstruído a partir da fonte) porque precisava ser corrigido por algum motivo, documentamos o patch em um documento separado, onde listamos essas "advertências" para referência futura, e o código fonte geralmente conterá um comentário semelhante a:

// [PATCH_START:product_name]
//  ... real code here ...
// [PATCH_END:product_name]

Caso 3 - Correções não óbvias

Este é um pouco mais controverso e mais próximo do que seu desenvolvedor sênior está pedindo.

No produto em que trabalho no momento, às vezes (definitivamente não é uma coisa comum) temos um comentário como:

// BUGFIX: [2010-12-09 haylem] fix for BUG_ID-XYZ

Só fazemos isso se a correção de bug não for óbvia e o código for lido de maneira anormal. Pode ser o caso de peculiaridades do navegador, por exemplo, ou correções obscuras de CSS que você precisa implementar apenas porque há um erro no documento em um produto. Portanto, em geral, o vincularíamos ao nosso repositório de problemas internos, que conterá o raciocínio detalhado por trás da correção de bugs e os ponteiros para a documentação do bug do produto externo (por exemplo, um aviso de segurança para um defeito conhecido do Internet Explorer 6 ou algo parecido).

Mas, como mencionado, é bastante raro. E graças às tags de tarefa, podemos executá-las regularmente e verificar se essas correções estranhas ainda fazem sentido ou podem ser eliminadas gradualmente (por exemplo, se deixamos de lado o suporte ao produto com bugs que causou o bug).


Isto apenas em: Um exemplo da vida real

Em alguns casos, é melhor que nada :)

Acabei de encontrar uma enorme classe de computação estatística em minha base de código, onde o comentário do cabeçalho estava na forma de um registro de alterações com o usual yadda yadda: revisor, data, ID do bug.

No começo, pensei em desmantelar, mas notei que os códigos de bug não correspondiam não apenas à convenção de nosso rastreador de problemas atual, mas também ao do rastreador usado antes de eu ingressar na empresa. Então, tentei ler o código e entender o que a classe estava fazendo (não sendo estatístico) e também tentei desenterrar esses relatórios de defeitos. Por acaso, eles eram bastante importantes e teriam atrapalhado a vida do próximo cara para editar o arquivo sem conhecê-los bastante horrível, pois tratava de pequenos problemas de precisão e casos especiais baseados em requisitos muito específicos emitidos pelo cliente de origem na época. . Resumindo, se estes não estivessem lá, eu não saberia. Se eles não estivessem lá E eu tivesse uma melhor compreensão da classe,

Às vezes, é difícil acompanhar requisitos muito antigos como esses. No final, o que eu fiz ainda foi remover o cabeçalho, mas depois de entrar em um comentário em bloco antes de cada função incriminadora, descrevendo por que essas computações "estranhas" são solicitações específicas.

Então, nesse caso, eu ainda as considerava uma prática ruim, mas, felizmente, o desenvolvedor original os colocou! Teria sido melhor comentar o código claramente, mas acho que era melhor que nada.

haylem
fonte
Essas situações fazem sentido. Obrigado pela contribuição.
Joshua Smith
o segundo caso é um comentário de código comum, por que há um patch no código. o primeiro caso é válido: apenas uma observação de que o código não está concluído ou que há mais trabalho a fazer nessa parte.
Salandur
Bem, o desenvolvedor sênior do meu local de trabalho (eu) diz que as alterações ocorrem nos comentários de confirmação do seu sistema de gerenciamento de configuração de software. Você tem o seu SCM e rastreadores de defeitos conectados, certo? (google SCMBUG) Não faça manualmente o que pode ser automatizado.
Tim Williscroft
@ Tim: Bem, eu concordo com você, como diz a primeira linha da resposta. Mas, em casos não óbvios, os programadores (por preguiça, presumo, pois não querem perder tempo com isso) não verificarão os logs de confirmação e perderão algumas informações importantes, enquanto um comentário de 10 caracteres pode apontá-los para o ID do bug correto, que conterá os logs das revisões referentes a esse bug se você configurou seu SCM e rastreador para trabalharem juntos. Melhor de todos os mundos, na verdade.
21711 haylem
Na minha experiência, as mensagens de confirmação geralmente são omitidas (apesar das políticas para incluí-las, se houver alguma) ou são inúteis (apenas um número de problema e, geralmente, o número errado). Essas mesmas pessoas, no entanto, deixarão comentários no código ... Eu prefiro ter essas mensagens e nenhuma mensagem de confirmação do que nada (e em muitos ambientes em que trabalhei para obter as mensagens de confirmação / histórico de origem era tão difícil que era quase impossível , incluindo a necessidade de solicitar fontes de outras pessoas porque o acesso ao controle de versão era limitado "por motivos de segurança").
Jwenting
3

Usamos essa prática para arquivos que não estão sob controle de versão. Temos programas de RPG que rodam em um mainframe e provou ser difícil fazer muito mais do que fazer backup deles.

Para código fonte com versão, usamos as notas de check-in. Eu acho que é onde eles pertencem, não bagunçando o código. São metadados, afinal.

Michael K
fonte
Concordo que os arquivos que não estão sob controle de versão são uma história diferente.
Joshua Smith
1
"provou ser difícil fazer muito mais do que fazer backup deles". Não é realmente uma desculpa que meu CTO suportaria.
Tim Williscroft
@ Tim: Sempre aberto a sugestões - eu posso chutá-las na cadeia de comando. :) É difícil trabalhar com o mainframe para ativar e desativar os arquivos.
Michael K
Minha sugestão - presumivelmente você pode obtê-los (backup); por que não despejar isso em algum lugar e fazer um pequeno script fazer alterações mercuriais todas as noites?
Wyatt Barnett
2

Costumávamos ter essa prática há muito tempo em uma loja de SW, onde nosso gerente insistia que ele queria poder ler arquivos de código em papel e seguir seu histórico de revisões. Escusado será dizer que não me lembro de nenhuma ocasião concreta quando ele realmente olhou para uma impressão do arquivo de origem: - /

Felizmente, meus gerentes desde então têm mais conhecimento sobre o que os sistemas de controle de versão podem fazer (devo acrescentar que usamos o SourceSafe nessa primeira loja). Portanto, não adiciono metadados de versão ao código.

Péter Török
fonte
2

Geralmente, não é necessário se o seu SCM e IDE permitir que você use o recurso "show anotot" (chamado de qualquer maneira no Eclipse), você poderá ver facilmente quais commit (s) alteraram um pedaço de código e os comentários do commit devem dizer quem e por que.

A única vez que eu colocaria um comentário como esse no código é se é um pedaço de código particularmente estranho que pode causar confusão a alguém no futuro, comentarei brevemente com o número do bug para que eles possam ir ao relatório de erros e ler em detalhes sobre isso.

Alva
fonte
"você não deve entender isso" provavelmente é um comentário ruim para colocar no código. Mais uma vez, direi vincular seu SCM e rastreadores de erros.
Tim Williscroft
2

Obviamente, é quase garantido que esses comentários sejam incorretos ao longo do tempo; se você editar uma linha duas vezes, você adiciona dois comentários? Onde eles vão? Portanto, um processo melhor é confiar em suas ferramentas.

Isso é um sinal de que, por qualquer motivo, o desenvolvedor sênior não pode confiar no novo processo para rastrear alterações e conectar alterações ao sistema de rastreamento de problemas. Você deve lidar com esse desconforto para resolver o conflito.

Você precisa entender por que ele não pode confiar nisso. São velhos hábitos? Uma má experiência com o sistema SCM? Um formato de apresentação que não funciona para ele? Talvez ele nem saiba sobre ferramentas como 'git blame' e a exibição da linha do tempo do Perforce (que mostra isso essencialmente, embora possa não mostrar qual problema desencadeou a mudança).

Sem entender sua razão para o requisito, você não será capaz de convencê-lo.

Alex Feinman
fonte
2

Trabalho em um produto Windows com mais de 20 anos de idade. Temos uma prática semelhante que existe há muito tempo. Não sei contar quantas vezes essa prática salvou meu bacon.

Eu concordo que algumas coisas são redundantes. Os desenvolvedores costumavam usar essa prática para comentar o código. Quando analiso isso, digo para eles prosseguirem e excluirem o código, e o comentário não é necessário.

Mas, muitas vezes, é possível ver o código que já existe há uma década e modificado por 20 desenvolvedores, mas nunca refatorado. Todo mundo esqueceu há muito tempo todos os detalhes de requisitos que estavam no código original, independentemente de todas as alterações e não há documentação confiável.

Um comentário com um número de defeito fornece um local para procurar a origem do código.

Então, sim, o sistema SCM faz muito, mas não tudo. Trate-os como faria com qualquer outro comentário e adicione-os sempre que uma alteração significativa for feita. O desenvolvedor que analisa seu código há mais de 5 anos, como agradecerá.


fonte
1

Mencionar todas as alterações nos comentários é inútil se você tiver um VC. É útil mencionar um bug específico ou outra observação importante relacionada a uma linha específica . Uma mudança pode incluir muitas linhas alteradas, todas sob a mesma mensagem de confirmação.

9000
fonte
1

Não que eu possa dizer.

Eu divido os TODOs no meu código à medida que avanço, e o objetivo é removê-los no momento da revisão.

Paul Nathan
fonte