Есть ли основания оставлять маркеры конфликта в проверенном коде?

14

Рассмотрим маркеры конфликта. то есть:

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

В конкретном случае, который побудил меня опубликовать этот вопрос, ответственный член команды только что завершил слияние из основной ветки разработки в нашу ветку и в некоторых случаях оставил их в виде комментариев как своего рода документацию к тому, что только что было решена. Он оставил его в скомпилированном состоянии, тесты пройдены, так что все не так плохо, как вы думаете.

Инстинктивно, хотя, я действительно возражал против этого, однако, будучи дьяволом, защищающим себя, я понимаю, почему он мог это сделать:

  • потому что это подчеркивает другим разработчикам команды, что изменилось в результате слияния.
  • потому что те, кто более опытен с определенными частями кода, могут решить проблемы, проиллюстрированные в комментариях, чтобы он не догадывался.
  • потому что объединение в восходящем направлении - это правильная боль, и может быть трудно оправдать время, чтобы решить все правильно и полностью, поэтому необходимо некоторое неполное уведомление FIXME, так почему бы не использовать исходный конфликт в качестве комментария для документирования этого.

Мое возражение было инстинктивным, но я хотел бы иметь возможность обосновать его рационально или увидеть свою позицию более разумно. Может ли кто-нибудь привести мне примеры или даже опыт, когда люди плохо проводили время с кем-то, делающим это, и / или объясняли, почему это плохая практика (или вы можете сыграть адвоката дьявола и поддержать его).

Моя непосредственная обеспокоенность заключалась в том, что было бы неприятно, если бы я редактировал один из соответствующих файлов, вытащил изменения, получил реальные конфликты, но также добавил комментарии. Тогда у меня действительно был бы очень грязный файл. К счастью, этого не произошло.

Бенедикт
источник
1
Что это за система контроля версий?
с69
Вы уверены, что они были оставлены по ошибке? Возможно, кто-то пошел на просмотр diff и спас без слияния конфликтов. Я видел, как это случилось со SmartSVN раньше
CamelBlues,
1
мерзавец. Извините, я не отметил, потому что я не чувствовал, что фактический VCS был уместен. Они были намеренно проверены, несколько, в нескольких файлах. Я бы согласился, что случайное прощается раз или два.
Бенедикт
«Если бы я редактировал один из соответствующих файлов, вытащил изменения, получил реальные конфликты, но также и закомментировал. Тогда у меня действительно был бы очень грязный файл». Звучит в значительной степени эквивалентно комментариям, как // MatrixFrog 10/25/2011: Updated this function to fix bug #1234. Если я вижу подобные вещи, я думаю: «Что? Вот для чего git blame
MatrixFrog

Ответы:

27

Это явно неправильно. Задача системы контроля версий - отслеживать изменения, а задача инструментов diff - показать, что изменилось в результате слияния. В журнале коммитов и, возможно, в коде должен быть комментарий, объясняющий, что было изменено и почему. Тем не менее, IMHO, оставлять маркеры конфликта в качестве комментариев - это то же самое, что оставлять мертвый код.

Дима
источник
5

У меня была похожая проблема с некоторым кодом, либо закомментированным (что как-то похоже на ваш случай), так и перемещенным в метод, который фактически нигде не вызывается. Когда его спросили, почему люди делают это, он ответил, что чувствует себя немного безопаснее, когда у них еще есть какой-то блок кода. Наиболее очевидным контраргументом является то, что это работа VCS, а не их. Однако есть и еще один аспект. Когда кто-то еще читает код во время обучения или внесения изменений, он, вероятно, будет зацикливаться на таком комментарии. Он обязательно прочитает его и, возможно, потратит некоторое время, чтобы понять, почему он здесь и какое возможное отношение это имеет к его текущей работе. Поскольку маркер конфликта является признаком конфликта, который уже разрешен, это, безусловно, пустая трата времени.

Яцек Прусия
источник
4

Я думаю, что комментарии должны ссылаться на код, который там есть, а не на код, который был там в прошлом, или на события, которые произошли с кодом в прошлом, или на код, который существовал в параллельной вселенной (другой ветви) в мимо. Оставление маркеров так, как это делал член вашей команды, создает как минимум три проблемы:

  1. Исходный код, вероятно, был примерно таким blah blah null, и в отчете об ошибке говорилось: «Нельзя использовать ноль там, использовать то или иное, или что-то еще». Таким образом, два человека независимо исправили ошибку, и когда исправления были объединены, возник конфликт. Теперь в комментариях описывается не то, в чем была проблема, ни то, что исправлено, а только то, что в какой-то момент в прошлом было два разных исправления. Это не очень полезно. Подобный комментарий //blah blah needs a non-null argument, по крайней мере, дает представление о том, что изменилось (и даже эта информация легче доступна из комментария коммита системы контроля версий).
  2. Объединенная версия может даже не выглядеть как одна из оригинальных строк. Может быть, если вы хотите, чтобы бла-бла принимал то и это, правильная форма blah blah (this,that)или даже что-то более сложное. В этом случае, оставив сообщение о конфликте в качестве комментария, вы наверняка запутаете любого, кто попытается прочитать код позже.
  3. Большинство систем контроля версий дают вам доступ к истории проекта. Например, я могу щелкнуть правой кнопкой мыши файл в eclipse (с svn), сказать «Показать историю ...», а затем сказать «Сравнить текущий с ...» и получить окно сравнения, которое выделяет различия. легче понять, если в подсвечивании различий содержатся реальные различия, а не комментарии вокруг них. Каждый бит нефункционального изменения в коде затрудняет чтение различий.
Валленборн
источник
-1

Насколько раздражают маркеры конфликта в проверенном коде?

Так раздражает.

Apocalisp
источник
1
Извините, но этот ответ ничего не добавляет. Это должен был быть в лучшем случае комментарий.
Адам Лир