Existe-t-il une justification pour laisser des marqueurs de conflit dans le code enregistré?

14

Considérez les marqueurs de conflit. c'est à dire:

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

Dans le cas particulier qui m'a motivé à poster cette question, le responsable de l'équipe venait de réaliser une fusion de l'amont vers notre agence, et dans certains cas les avait laissés, en commentaires, comme une sorte de documentation sur ce qui venait d'être résolu. Il l'a laissé dans un état compilé, les tests ont réussi, donc ce n'est pas aussi mauvais qu'on pourrait le penser.

Cependant, instinctivement, je me suis vraiment opposé à cela, mais étant moi-même défenseur des démons, je peux voir pourquoi il aurait pu le faire:

  • car il met en évidence pour les autres développeurs d'équipe ce qui a changé à la suite de la fusion.
  • parce que ceux qui sont plus experts avec les morceaux de code particuliers peuvent alors répondre aux préoccupations illustrées par les commentaires afin qu'il n'ait pas à deviner.
  • car la fusion en amont est une vraie douleur et il peut être difficile de justifier le temps de tout résoudre correctement et complètement, donc un avis FIXME semi-complet est nécessaire, alors pourquoi ne pas utiliser le conflit d'origine comme commentaire pour documenter cela.

Mon objection était instinctive, mais j'aimerais pouvoir la justifier rationnellement, ou voir ma position plus sagement. Quelqu'un peut-il me donner des exemples ou même des expériences où les gens ont eu du mal avec quelqu'un d'autre à faire cela et / ou les raisons pour lesquelles c'est une mauvaise pratique (ou vous pouvez jouer l'avocat du diable et le soutenir).

Ma propre préoccupation immédiate était que cela aurait clairement été ennuyeux si j'avais édité l'un des fichiers concernés, retiré les modifications, obtenu de véritables conflits, mais aussi intégré les commentaires. J'aurais alors eu un dossier très salissant. Heureusement, cela ne s'est pas produit.

Benoît
la source
1
De quel système de contrôle de version s'agit-il?
c69
Êtes-vous sûr qu'ils ont été laissés par erreur? Peut-être que quelqu'un est allé voir diff et a sauvé sans fusionner les conflits. J'ai déjà vu cela se produire avec SmartSVN
CamelBlues
1
git. Désolé, je n'ai pas tagué parce que je ne pensais pas que le VCS était pertinent. Ils ont été volontairement enregistrés, plusieurs, dans plusieurs dossiers. Je conviens qu'un accidentel est pardonnable une ou deux fois.
Benoît
"Si j'avais édité l'un des fichiers concernés, retiré les modifications, obtenu de vrais conflits, mais aussi récupéré ceux commentés. Alors j'aurais eu un fichier très compliqué." Cela semble à peu près équivalent à des commentaires comme // MatrixFrog 10/25/2011: Updated this function to fix bug #1234. Si je vois des trucs comme ça, je pense: "Quoi? C'est pour ça git blame!"
MatrixFrog

Réponses:

27

C'est clairement faux. C'est le travail du système de contrôle de version de garder une trace des changements, et c'est le travail des outils de diff pour montrer ce qui a changé à la suite de la fusion. Il devrait y avoir un commentaire dans le journal de validation, et peut-être dans le code, expliquant ce qui a été changé et pourquoi. Cependant, à mon humble avis, laisser les marqueurs de conflit en tant que commentaires revient à laisser le code mort.

Dima
la source
5

J'ai eu un problème similaire avec du code commenté (ce qui est en quelque sorte similaire à votre cas) ou déplacé vers une méthode qui n'est en fait appelée nulle part. Lorsqu'on leur a demandé pourquoi les gens faisaient cela, la réponse a été qu'ils se sentent un peu plus en sécurité lorsqu'ils ont encore un bloc de code. Le contre-argument le plus évident est que c'est un travail VCS et non le leur. Mais il y a aussi un autre aspect. Quand quelqu'un d'autre lit du code tout en apprenant ou en apportant des modifications, il sera probablement mis de côté par un tel commentaire. Il le lira certainement et passera peut-être un peu de temps à comprendre pourquoi il est ici et quelle corrélation possible il a avec son emploi actuel. Étant donné qu'un marqueur de conflit est le signe d'un conflit qui a déjà été résolu, c'est certainement une perte de temps.

Jacek Prucia
la source
4

Je pense que les commentaires devraient se référer au code qui est là, pas au code qui a été là dans le passé, ni aux événements qui sont arrivés au code à un moment donné dans le passé, ni au code qui existait dans un univers parallèle (une autre branche) dans le passé. Laisser les marqueurs comme le faisait votre membre d'équipe crée au moins trois problèmes:

  1. Le code d'origine ressemblait probablement à quelque chose comme ça blah blah null, et le rapport de bogue disait "Ne peut pas utiliser null là, utiliser ceci ou cela, ou quoi que ce soit." Donc, deux personnes ont corrigé le bogue de manière indépendante et lorsque les correctifs ont été fusionnés, le conflit est survenu. Maintenant, le commentaire ne documente pas quel était le problème ni quel correctif a été corrigé, mais seulement qu'il y avait deux correctifs différents à un moment donné dans le passé. Ce n'est pas très utile. Un commentaire comme //blah blah needs a non-null argumentdonnerait au moins une indication de ce qui a changé (et même cette information est plus facilement disponible à partir du commentaire de validation du système de contrôle de version).
  2. La version fusionnée pourrait même ne pas ressembler à l'une des lignes d'origine. Peut-être que si vous voulez que bla bla prenne ceci et cela, la forme correcte est blah blah (this,that)ou même quelque chose de plus compliqué. Dans ce cas, laisser le message de conflit en tant que commentaire va sûrement dérouter quiconque essaie de lire le code plus tard.
  3. La plupart des systèmes de contrôle de version vous donnent accès à l'historique du projet. Par exemple, je peux cliquer avec le bouton droit sur un fichier dans éclipse (avec svn), dire "Afficher l'historique ..." puis dire "Comparer le courant avec ..." et obtenir une fenêtre de différence qui met en évidence les différences. plus facile à comprendre si les surbrillances diff contiennent les différences réelles, et non les commentaires qui les entourent. Chaque changement non fonctionnel dans le code rend cette différence plus difficile à lire.
Wallenborn
la source
-1

Dans quelle mesure les marqueurs de conflit sont-ils gênants dans le code enregistré?

Tellement ennuyeux.

Apocalisp
la source
1
Je suis désolé mais cette réponse n'ajoute vraiment rien. Cela aurait dû être au mieux un commentaire.
Adam Lear