Ist es gerechtfertigt, Konfliktmarkierungen im eingecheckten Code zu belassen?

14

Betrachten Sie Konfliktmarkierungen. dh:

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

In dem speziellen Fall, der mich veranlasst hat, diese Frage zu stellen, hat das zuständige Teammitglied gerade eine Zusammenführung von Upstream zu unserer Niederlassung durchgeführt und diese in einigen Fällen als Kommentar als eine Art Dokumentation über das, was gerade war, hinterlassen behoben. Er hat es in einem kompilierten Zustand belassen und die Tests bestanden. Es ist also nicht so schlimm, wie man denkt.

Instinktiv nahm ich dies jedoch wirklich in Abrede, aber als Teufel, der sich für mich einsetzt, kann ich sehen, warum er es getan haben könnte:

  • weil es anderen Teamentwicklern aufzeigt, was sich durch die Zusammenführung geändert hat.
  • weil diejenigen, die mit den einzelnen Codeteilen besser vertraut sind, dann auf die Bedenken eingehen können, die in den Kommentaren verdeutlicht werden, damit er nicht raten muss.
  • Da die vorgelagerte Zusammenführung ein Problem darstellt und es schwierig sein kann, die Zeit zu rechtfertigen, in der alles gut und vollständig gelöst werden muss, ist eine halbfertige FIXME-Benachrichtigung erforderlich. Verwenden Sie den ursprünglichen Konflikt als Kommentar, um dies zu dokumentieren.

Mein Einwand war instinktiv, aber ich möchte ihn rational rechtfertigen oder meine Position klüger sehen können. Kann mir jemand einige Beispiele oder sogar Erfahrungen geben, bei denen die Leute eine schlechte Zeit mit jemand anderem hatten und / oder Gründe, warum dies eine schlechte Praxis ist (oder Sie können den Anwalt des Teufels spielen und ihn unterstützen).

Meine unmittelbare Sorge war, dass es eindeutig ärgerlich gewesen wäre, wenn ich eine der betroffenen Dateien bearbeitet, die Änderungen übernommen, echte Konflikte festgestellt, aber auch die kommentierten eingezogen hätte. Dann hätte ich in der Tat eine sehr unordentliche Akte gehabt. Zum Glück ist das nicht passiert.

Benedikt
quelle
1
Welches Versionskontrollsystem ist das?
c69
Sind Sie sicher, dass diese versehentlich zurückgelassen wurden? Vielleicht hat sich jemand diff angesehen und gespeichert, ohne die Konflikte zusammenzuführen. Ich habe dies schon einmal mit SmartSVN gesehen
CamelBlues
1
git. Tut mir leid, dass ich nicht getaggt habe, weil ich nicht der Meinung war, dass das tatsächliche VCS relevant ist. Sie wurden absichtlich in mehreren Akten eingecheckt. Ich würde zustimmen, dass ein oder zwei Versehen vergeben werden können.
Benedikt
"Wenn ich eine der betroffenen Dateien bearbeitet hätte, die Änderungen übernommen hätte, echte Konflikte erhalten hätte, aber auch die kommentierten eingezogen hätte. Dann hätte ich in der Tat eine sehr unordentliche Datei." Klingt so ziemlich gleichbedeutend mit Kommentaren wie // MatrixFrog 10/25/2011: Updated this function to fix bug #1234. Wenn ich so etwas sehe, denke ich: "Was? Dafür git blameist es da!"
MatrixFrog

Antworten:

27

Das ist eindeutig falsch. Es ist die Aufgabe des Versionskontrollsystems, Änderungen im Auge zu behalten, und es ist die Aufgabe von Diff-Tools, um anzuzeigen, was sich durch die Zusammenführung geändert hat. Im Festschreibungsprotokoll und möglicherweise im Code sollte ein Kommentar enthalten sein, in dem erläutert wird, was und warum geändert wurde. Allerdings ist es meiner Meinung nach dasselbe, die Konfliktmarkierungen als Kommentare zu belassen, als würde man toten Code herumstehen lassen.

Dima
quelle
5

Ich hatte ein ähnliches Problem damit, dass Code entweder auskommentiert wurde (was in Ihrem Fall ähnlich ist) oder zu einer Methode überging, die eigentlich nirgendwo aufgerufen wurde. Auf die Frage, warum die Leute das tun, antworteten sie, dass sie sich etwas sicherer fühlen, wenn sie noch einen Codeblock in der Nähe haben. Das offensichtlichste Gegenargument ist, dass es sich um einen VCS-Job handelt und nicht um ihren. Es gibt jedoch auch einen anderen Aspekt. Wenn jemand anderes Code liest, während er lernt oder Änderungen vornimmt, wird er wahrscheinlich von einem solchen Kommentar abgelenkt. Er wird es auf jeden Fall lesen und vielleicht einige Zeit damit verbringen, zu verstehen, warum es hier ist und welche mögliche Korrelation es zu seinem aktuellen Job hat. Da ein Konfliktmarker ein Zeichen für einen bereits gelösten Konflikt ist, ist dies mit Sicherheit eine Zeitverschwendung.

Jacek Prucia
quelle
4

Ich denke, Kommentare sollten sich auf den Code beziehen, der dort vorhanden ist, nicht auf Code, der in der Vergangenheit vorhanden war, noch auf Ereignisse, die mit dem Code in der Vergangenheit passiert sind, oder auf Code, der in einem parallelen Universum (einem anderen Zweig) in der Datenbank vorhanden war Vergangenheit. Wenn Sie die Marker so belassen, wie es Ihr Teammitglied getan hat, treten mindestens drei Probleme auf:

  1. Der ursprüngliche Code war wahrscheinlich so ähnlich blah blah null, und im Fehlerbericht stand "Kann dort nicht null verwenden, benutze dies oder das oder was auch immer." Zwei Personen haben den Fehler unabhängig voneinander behoben, und als die Korrekturen zusammengeführt wurden, kam es zu einem Konflikt. Jetzt dokumentiert der Kommentar nicht, was das Problem war oder was der Fix behoben hat, sondern nur, dass es zu irgendeinem Zeitpunkt in der Vergangenheit zwei verschiedene Fixes gab. Das ist nicht sehr hilfreich. Ein Kommentar wie //blah blah needs a non-null argumentwürde zumindest einen Hinweis darauf geben, was sich geändert hat (und selbst diese Informationen sind im Commit-Kommentar des Versionskontrollsystems leichter verfügbar).
  2. Die zusammengeführte Version sieht möglicherweise nicht einmal wie eine der ursprünglichen Zeilen aus. Vielleicht, wenn du willst, dass bla bla dies und das nimmt, ist die richtige Form blah blah (this,that)oder sogar etwas Komplizierteres. In diesem Fall wird das Hinterlassen der Konfliktnachricht als Kommentar sicherlich jeden verwirren, der später versucht, den Code zu lesen.
  3. Die meisten Versionskontrollsysteme bieten Zugriff auf die Projekthistorie. Zum Beispiel kann ich mit der rechten Maustaste auf eine Datei in Eclipse (mit SVN) klicken, "Show History ..." und dann "Compare Current with ..." sagen und ein Diff-Fenster erhalten, das die Unterschiede hervorhebt Dies ist einfacher zu verstehen, wenn die diff-Hervorhebungen die tatsächlichen Unterschiede enthalten und nicht die Kommentare, die sie umgeben.
Wallenborn
quelle
-1

Wie ärgerlich sind Konfliktmarkierungen im eingecheckten Code?

So nervig.

Apocalisp
quelle
1
Es tut mir leid, aber diese Antwort fügt nichts hinzu. Es hätte bestenfalls ein Kommentar sein sollen.
Adam Lear