Existe justificativa para deixar marcadores de conflito no código de check-in?

14

Considere marcadores de conflito. ou seja:

<<<<<<< branch
blah blah this
=======
blah blah that
>>>>>>> HEAD

No caso específico que me motivou a postar essa pergunta, o membro da equipe responsável havia acabado de concluir uma mesclagem do montante para a nossa filial e, em alguns casos, os deixou como comentários, como uma espécie de documentação sobre o que acabara de ser apresentado. resolvido. Ele deixou em estado compilado, os testes foram aprovados, para que não seja tão ruim quanto você imagina.

Instintivamente, porém, eu realmente me opus a isso, por mais que os demônios se defendam, posso ver por que ele pode ter feito isso:

  • porque destaca para outros desenvolvedores da equipe o que mudou como resultado da mesclagem.
  • porque aqueles que são mais especialistas em partes específicas do código podem resolver as preocupações ilustradas pelos comentários, para que ele não precise adivinhar.
  • porque a mesclagem upstream é uma dor certa e pode ser difícil justificar o tempo para resolver tudo bem e completamente, por isso é necessário algum aviso FIXME semi-completo, por que não usar o conflito original como um comentário para documentar isso.

Minha objeção foi instintiva, mas eu gostaria de justificá-la racionalmente ou ver minha posição com mais sabedoria. Alguém pode me dar alguns exemplos ou mesmo experiências em que as pessoas tiveram um mau momento com alguém fazendo isso e / ou razões pelas quais é uma má prática (ou você pode bancar o advogado do diabo e apoiá-lo).

Minha preocupação imediata foi que seria claramente irritante se eu estivesse editando um dos arquivos em questão, puxasse as alterações, tivesse conflitos reais, mas também os comentados. Então eu teria realmente um arquivo muito confuso. Felizmente isso não aconteceu.

Bento
fonte
1
Que sistema de controle de versão é esse?
C69
Tem certeza de que foram deixados por engano? Talvez alguém tenha visualizado o diff e economizado sem a fusão dos conflitos. Eu já vi isso acontecer com o SmartSVN antes
CamelBlues 25/10
1
git. Desculpe, mas não marquei a etiqueta porque não senti o VCS real relevante. Eles foram deliberadamente registrados, vários, em vários arquivos. Concordo que acidental é perdoável uma ou duas vezes.
Bento
"se eu estivesse editando um dos arquivos em questão, extraísse as alterações, obtivesse conflitos reais, mas também extraísse os comentados. Então, eu teria um arquivo muito confuso". Parece bastante equivalente a comentários como // MatrixFrog 10/25/2011: Updated this function to fix bug #1234. Se eu vejo coisas assim, penso: "O quê? É git blamepara isso!"
MatrixFrog

Respostas:

27

Isto está claramente errado. O trabalho do sistema de controle de versão é acompanhar as alterações e o trabalho das ferramentas diff para mostrar o que mudou como resultado da mesclagem. Deve haver um comentário no log de confirmação, e talvez no código, explicando o que foi alterado e por quê. No entanto, IMHO, deixar os marcadores de conflito como comentários é o mesmo que deixar o código morto por aí.

Dima
fonte
5

Eu tive um problema semelhante com algum código sendo comentado (o que é de alguma forma semelhante ao seu caso) ou movido para um método que não é realmente chamado em qualquer lugar. Quando perguntados por que as pessoas fazem isso, a resposta foi que elas se sentem um pouco mais seguras quando ainda têm algum bloco de código por perto. O contra-argumento mais óbvio é que é um trabalho VCS e não o deles. No entanto, há também outro aspecto. Quando alguém está lendo código enquanto aprende ou faz alterações, ele provavelmente será desviado por esse comentário. Definitivamente, ele o lerá e, talvez, dedique algum tempo para entender por que está aqui e qual a possível correlação que ele tem com seu trabalho atual. Como um marcador de conflito é um sinal de um conflito que já foi resolvido, isso é certamente uma perda de tempo.

Jacek Prucia
fonte
4

Acho que os comentários devem se referir ao código que está lá, não ao código que existia no passado, nem aos eventos que aconteceram com o código em algum momento do passado, nem ao código que existia em um universo paralelo (outro ramo) no passado. Deixar os marcadores da maneira que o membro da equipe criou cria pelo menos três problemas:

  1. O código original provavelmente era algo assim blah blah null, e o relatório de erro dizia "Não é possível usar nulo lá, use isto ou aquilo, ou o que for". Portanto, duas pessoas corrigiram o bug independentemente e, quando as correções foram mescladas, o conflito surgiu. Agora, o comentário documenta não qual era o problema nem a correção corrigida, mas apenas que havia duas correções diferentes em algum momento no passado. Isso não é muito útil. Um comentário como //blah blah needs a non-null argumentpelo menos indicaria o que mudou (e mesmo essa informação está mais facilmente disponível no comentário de confirmação do sistema de controle de versão).
  2. A versão mesclada pode nem parecer uma das linhas originais. Talvez se você quiser que blá-blá aceite isso e aquilo, a forma correta é blah blah (this,that)ou mesmo algo mais complicado. Nesse caso, deixar a mensagem de conflito como um comentário certamente confundirá quem tentar ler o código posteriormente.
  3. A maioria dos sistemas de controle de versão fornece acesso ao histórico do projeto. Por exemplo, posso clicar com o botão direito do mouse em um arquivo no eclipse (com svn), dizer "Mostrar histórico ..." e dizer "Comparar atual com ..." e obter uma janela de diferenças que destaca as diferenças. É mais fácil entender se os destaques do diff contêm as diferenças reais, e não os comentários ao seu redor.Todas as alterações não funcionais no código tornam esse diff mais difícil de ler.
wallenborn
fonte
-1

Quão irritantes são os marcadores de conflito no código de check-in?

Tão irritante.

Apocalisp
fonte
1
Sinto muito, mas esta resposta realmente não adiciona nada. Deveria ter sido, na melhor das hipóteses, um comentário.
Adam Lear