Czy istnieje uzasadnienie dla pozostawienia znaczników konfliktu w zameldowanym kodzie?

14

Rozważ znaczniki konfliktu. to znaczy:

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

W szczególnym przypadku, który zmotywował mnie do opublikowania tego pytania, odpowiedzialny członek zespołu właśnie zakończył scalenie z wyższego szczebla do naszego oddziału, aw niektórych przypadkach pozostawił je jako komentarz, jako rodzaj dokumentacji dotyczącej tego, co właśnie zostało zdecydowany. Pozostawił go w stanie skompilowanym, testy zdały, więc nie jest tak źle, jak mogłoby się wydawać.

Instynktownie jednak naprawdę sprzeciwiłem się temu, jednak będąc diabłami, które mnie popierają, rozumiem, dlaczego mógł to zrobić:

  • ponieważ podkreśla innym programistom zespołów, co zmieniło się w wyniku scalenia.
  • ponieważ ci, którzy są bardziej biegli w poszczególnych fragmentach kodu, mogą następnie rozwiązać obawy przedstawione w komentarzach, aby nie musiał zgadywać.
  • ponieważ scalanie w górę łańcucha jest właściwym bólem i może być trudne uzasadnienie czasu, aby rozwiązać wszystko dobrze i całkowicie, dlatego konieczne jest pewne niepełne powiadomienie FIXME, więc dlaczego nie wykorzystać oryginalnego konfliktu jako komentarza do udokumentowania tego.

Mój sprzeciw był instynktowny, ale chciałbym móc to racjonalnie uzasadnić lub mądrzej zobaczyć moją pozycję. Czy ktoś może mi podać kilka przykładów, a nawet doświadczeń, w których ludzie źle się z tym robią i / lub powody, dla których jest to zła praktyka (lub możesz wcielić się w adwokata diabła i wesprzeć go).

Moją bezpośrednią obawą było to, że byłoby to irytujące, gdybym edytował jeden z odnośnych plików, wyciągnąłem zmiany, dostałem prawdziwe konflikty, ale także wciągnąłem komentarze. Wtedy miałbym naprawdę bardzo niechlujny plik. Na szczęście tak się nie stało.

Benedykt
źródło
1
Co to za system kontroli wersji?
c69,
Czy jesteś pewien, że zostały one pomyłkowo? Może ktoś poszedł zobaczyć różnice i uratował bez połączenia konfliktów. Widziałem to wcześniej w przypadku SmartSVN
CamelBlues
1
git. Przepraszam, że nie oznaczyłem tagiem, ponieważ nie czułem, aby rzeczywisty VCS był odpowiedni. Zostały celowo zameldowane, kilka, w kilku aktach. Zgodziłbym się, że przypadkowi można wybaczyć raz lub dwa razy.
Benedykt
„gdybym edytował jeden z odnośnych plików, wyciągnąłem zmiany, dostałem prawdziwe konflikty, ale także wciągnąłem te z komentarzami. Wtedy rzeczywiście miałbym bardzo niechlujny plik”. Brzmi prawie tak samo jak komentarze // MatrixFrog 10/25/2011: Updated this function to fix bug #1234. Jeśli widzę takie rzeczy, myślę: „Po co? Po to git blamejest!”
MatrixFrog,

Odpowiedzi:

27

To jest oczywiście złe. Zadaniem systemu kontroli wersji jest śledzenie zmian, a zadaniem narzędzi różnicowych jest pokazanie, co zmieniło się w wyniku scalenia. W dzienniku zatwierdzeń, a może i w kodzie, powinien znajdować się komentarz wyjaśniający, co zostało zmienione i dlaczego. Jednak IMHO pozostawianie znaczników konfliktu w komentarzach jest tym samym, co pozostawienie martwego kodu.

Dima
źródło
5

Miałem podobny problem z tym, że jakiś kod albo został skomentowany (co jest jakoś podobne do twojej sprawy) lub został przeniesiony do metody, która nigdzie nie jest wywoływana. Zapytani, dlaczego ludzie to robią, odpowiedzieli, że czują się nieco bezpieczniej, gdy nadal mają jakiś blok kodu. Najbardziej oczywistym kontrargumentem jest to, że jest to zadanie VCS, a nie ich. Istnieje jednak również inny aspekt. Gdy ktoś inny czyta kod podczas nauki lub wprowadzania zmian, prawdopodobnie będzie śledzony przez taki komentarz. Na pewno go przeczyta i być może poświęci trochę czasu, aby zrozumieć, dlaczego jest tutaj i jaką ewentualną korelację ma z jego obecną pracą. Ponieważ znacznik konfliktu jest znakiem konfliktu, który już został rozwiązany, jest to z pewnością strata czasu.

Jacek Prucia
źródło
4

Myślę, że komentarze powinny odnosić się do kodu, który tam jest, nie do kodu, który był tam w przeszłości, ani do wydarzeń, które zdarzyły się z kodem w przeszłości, ani do kodu, który istniał w równoległym wszechświecie (inna gałąź) w przeszłość. Pozostawienie znaczników w sposób, w jaki zrobił to członek twojego zespołu, stwarza co najmniej trzy problemy:

  1. Oryginalny kod prawdopodobnie był podobny blah blah null, a raport o błędzie powiedział: „Nie można użyć wartości null, użyć tego czy tamtego, ani nic innego”. Tak więc dwie osoby samodzielnie naprawiły błąd, a kiedy poprawki zostały scalone, powstał konflikt. Teraz komentarz nie dokumentuje, jaki był problem, ani tego, co naprawiono, ale tylko to, że w przeszłości istniały dwie różne poprawki. To nie jest bardzo pomocne. Komentarz taki //blah blah needs a non-null argumentdałby przynajmniej wskazówkę co się zmieniło (a nawet ta informacja jest łatwiej dostępna z komentarza zatwierdzenia systemu kontroli wersji).
  2. Wersja scalona może nawet nie wyglądać jak jedna z oryginalnych linii. Może jeśli chcesz, żeby bla bla brał to i tamto, poprawna forma jest, blah blah (this,that)a nawet bardziej skomplikowana. W takim przypadku pozostawienie komunikatu o konflikcie jako komentarza z pewnością dezorientuje każdego, kto spróbuje później odczytać kod.
  3. Większość systemów kontroli wersji zapewnia dostęp do historii projektu. Na przykład, mogę kliknąć prawym przyciskiem myszy plik w środowisku Eclipse (z svn), powiedzieć „Pokaż historię ...”, a następnie powiedzieć „Porównaj bieżący z ...” i wyświetlić okno różnic, które podkreśla różnice. łatwiejsze do odczytania, jeśli wyróżnienia różnic zawierają rzeczywiste różnice, a nie komentarze wokół nich. Każda drobna niefunkcjonalna zmiana w kodzie sprawia, że ​​różnice są trudniejsze do odczytania.
Wallenborn
źródło
-1

Jak denerwujące są znaczniki konfliktu w zameldowanym kodzie?

Tak denerwujące.

Apokalipsa
źródło
1
Przykro mi, ale ta odpowiedź tak naprawdę nic nie dodaje. To powinien być co najwyżej komentarz.
Adam Lear