14 февраля 2008 в 00:40

Subversion: чеклист по правильным коммитам

Предполагаем, что читатель: а) работает в коллективе; и б) осознал необходимость правильной работы с системами контроля версий или хотя бы поставлен перед необходимостью использовать таковую.

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

Грубо разделим фазы разработки проекта на три — дебют, миттельшпиль и эндшпиль.

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

В миттельшпиле система в целом стабилизировалась, продукт близится к релизу. Рефакторинг более чёткий, хотя и иногда достаточно обширный. На этом этапе ожидается уже, что система в целом работает — по крайней мере, сломанный репозиторий порицается.

Наконец, в эндшпиль система переходит непосредственно перед релизом и сразу после. Для веб-приложений постоянно добавляются новые относительно небольшие возможности, а крупные изменения сначала тестируются на ветках. Для более традиционных приложений наоборот, создаётся ветка для maintenance релизов, а разработка следующей большой версии продолжается на стволе.



В дебюте коммиты делаются относительно редко. Например, хорошей точкой коммита может стать создание нового класса, нового отношения или же написание предварительной версии достаточно большого куска кода. Для некоторых программистов, особенно высокопроизводительных, может быть даже удобным коммит, привязанный ко времени — два-три раза в день. Репозиторий может быть в достаточно «разобранном» состоянии, вплоть до того, что извлеченный код попросту не компилируется.

В миттельшпиле поток коммитов гораздо более структурирован. Коммиты чаще, и привязаны только к структурным изменениям. Коммиты в основном не больше среднего размера. Хорошими точками для коммита будут: закрытие таска или бага в трекере, рефакторинг, добавление или исправление очередной фичи. Репозиторий практически всегда находится в более-менее работоспособном состоянии, а сломанные репозитории порицаются (а сам факт ломания наносит кармический удар).

Наконец, в эндшпиле коммиты так или иначе контролируются достаточно жестко. Вообще сам факт работы над проектом в этой фазе должен иметь некоторое обоснование — обычно это баг или перенесенная за релиз важная фича. Коммиты на этой фазе достаточно быстро уходят на продакшн, поэтому они: чётко привязаны к баг-трекеру; относительно мелкие; хорошо документированы; хорошо оттестированы и продуманы — внесение дополнительных багов на этой стадии наносит серьёзный кармический удар).

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

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

Коммиты должны быть логичными. Коммит должен соответствовать одной структурной единице — новому файлу, новому блоку, новому инклюднику, новому классу, новому отношению, новой фиче, закрытому багу, одиночному рефакторингу, главе документации, исправлению документации и т. п.

Смысл этого требования состоит в игре на благо будущего. Соблюдая его, можно быть уверенным, тот или иной коммит в потоке либо безопасен (например, фикс документации), либо может быть свободно откачен с уверенностью, что откатилось в точности одно логическое изменения. Это бесценное свойство при бинарном поиске ошибок. Более того, очевидно, что логичные изменения гораздо проще отсматривать в процессе код-ревью.

Коммиты должны быть протестированными (за исключением дебютной фазы). Идеально если изменение вообще протестировано юнит-тестами. В крайнем случае коммит нужно хотя бы раз проверить — даже самые безобидные исправления могу привести к катастрофическим для кармы последствиям. Безусловно, коммит должен быть синтаксически валидным — фикс к фиксу наносит удар по имиджу.

Коммиты должны быть хорошо документированными. Мы молчим про пустые журнальные сообщения — это вообще худшее что может быть. Это допустимо только для совершенно далеких от программирования сотрудников (типа дизайнеров), которые на этот ваш сабвершен с трудом вообще согласились. И то, имеет смысл работать над культурным уровнем даже таких людей! Коммит с описанием «Fix» или «Fixed» допустим только если в диффе не более двух-трёх строчек. Если больше — надо писать более развернуто.

Отдельно надо рассмотреть вопрос журнального сообщения, состоящего из нескольких не связанных друг с другом пунктов. Понятно, что такое сообщение — это симптом истинной проблемы — той, что ваш коммит состоит из нескольких не связанных друг с другом частей. Лучшим решением в данном случае было бы отменить попытку коммита и постараться расщепить его на несколько частей.

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

Перед коммитом необходимо просматривать диффы, а также следить за новыми файлами, не добавленными в репозиторий, и добавлять их. Если вы пользуетесь TortoiseSVN, то в ней есть специальный пункт меню под названием «Check for modifications». В комманд-лайновом клиенте для этой цели служат команды svn st (нет ли болтающихся файлов) и svn diff | more (для отсмотра изменений перед коммитом).

Это правило необходимо соблюдать, чтобы не закоммитить случайно что-нибудь не то. Известно, что например в процессе исправления ошибок субъективно кажется, что «было много работы». Однако, при отсмотре диффа оказывается, что реально было исправлено одна-две строчки. Отсмотр также позволяет устранить мусор из коммитов — например, временные отладочные операторы и пустые изменения (добавление и удаление пробелов). Зачастую бывает, что в том или ином файле вообще остаются только пробельные изменения — их необходимо откатить в исходному состоянию (svn revert). В фазе эндшпиля отсмотр изменений перед коммитом строго обязателен.

Отдельно следует упомянуть об исправлении выравнивания и пробелов в файлах. Такая операция (если она сильно необходима) должна коммититься самостоятельно. В фазе эндшпиля ни в коем случае нельзя допускать коммита пробельных изменений вместе с существенными изменениями.

Также более мелкие правила:

— при переименовании файла и внесении в него изменений следует отдельно закоммитить переименование, а отдельно изменение;

— при добавлении в код проекта стороннего компонента (плагина Rails, файла prototype.js) их следует коммитить отдельно;

— не следует оставлять закомментированные строки кода — для хранения старых версий, натурально, есть система контроля версий (в некоторых случаях при одно-двухстрочных исправлениях это правило может нарушаться).

Вопросы, пожелания, предложения?

P.S. У нас есть целая куча вакансий: undev.ru
Алексей Махоткин @squadette
карма
82,2
рейтинг 0,0
Самое читаемое

Комментарии (59)

  • НЛО прилетело и опубликовало эту надпись здесь
    • +2
      fixed
  • НЛО прилетело и опубликовало эту надпись здесь
    • НЛО прилетело и опубликовало эту надпись здесь
      • +4
        ну это безусловно некоторый драфт
        но имхо, такое снисходительное "не умеющий ничего не поймёт" — лишнее. всё же если уж человек пришёл тут к нам в индустрию — пусть не рассказывает, что маску сварщика на улице нашел.

        не поймёт — в топку.
        • НЛО прилетело и опубликовало эту надпись здесь
          • 0
            (смотрит в http://vzasos.habrahabr.ru/blog/)

            (пожимает плечами)
            • НЛО прилетело и опубликовало эту надпись здесь
              • 0
                да нет, критика вся в тему, я в общем понимаю, что это до некоторой степени драфт кровью сердца.

                просто Вы раз за разом пытаетесь всё же добиться того, чтобы я к Вам прислушался. И раз за разом новая информация заставляет меня скорее делать нечто обратное :)

                это я ещё даже не стал пока читать, что там за статьи у Зубинского :)
        • 0
          Исконно русское слово драфт... А за статью спасибо. Вовремя.
    • +1
      От лица минимум одного "не умеющего" - всё понял и сделал выводы. Честно )
  • +2
    Хорошая статья, спасибо. Хоть и работаю с свном уже давно, и многие вещи считаю практически естественными, но кое-что подчерпнул. Щас добавим вам "кармической силы" =)
  • 0
    На счет коммента к коммиту с разрозненными пунктами: предполагается, что к такого рода комментам следует относиться уничтожающим вниманием на среднем и позднем этапах работы, так? Ведь на этапе дебюта можно, в самом деле, наворотить много делов и не факт, что они будет тесно связаны.
    • 0
      на третьем этапе строгое "нет"

      на втором — публичное унижение и оставление без завтрака
  • 0
    Спасибо. Почерпнул немного полезного. Есть рекомендации которые помогут в дальнейшем качественно хорошо работать в команде.
  • +1
    По diff перед коммитом. Ну так для себя чисто открыл colordiff и команда - "svn diff | colordiff | less -SR" - это вообще красота!!!

    Про переименование файла - вот тут не согласен с двумя коммитами, вполне возможно обойтись и одним - просто надо правильно переименовывать (svn move), а потом правим этот файл и коммитим. SVN в этом сдучае дифф между версиями покажет нормальный.

    Ну и в эндшпиле еще есть хорошая практика тэгов. То есть перед RC, или релизом версии (точнее во время) создается тэг - то есть непосредственно то что лежит на продакшене, что бы можно было с точностью взять код продакшена и попробовать воспроизвести на нём тот баг, который происходит.
    • 0
      в файле ~/.subversion/config можно определить diff-cmd = colordiff
      • 0
        ну я уже давно сделал алиас, но надо будет попробовать законфигурять так как вы говорите.
      • 0
        alias svndiff='svn diff | vim -'
        • 0
          алиас тем плох — что ломает рефлексы по набиранию svn di
          лучше конфигурить старые команды, чем учить новые
        • 0
          sdiff() {
          svn diff $1 | colordiff | less -SR
          }
    • 0
      svn diff | colordiff | less -SR
      Вообще супер !
    • 0
      Согласен про один коммит. Вообще в статье много утверждений, не подкреплённых аргументами.
      • 0
        они подкреплены опытом
        • 0
          Опыт подразумевает, что вы пытались делать иначе и наступили на грабли, но вместо того, чтобы их описать, вы предлагаете просто делать два коммита. Почему читатель должен верить вам на слово?
          • 0
            «Смысл этого требования состоит в игре на благо будущего. Соблюдая его, можно быть уверенным, тот или иной коммит в потоке либо безопасен (например, фикс документации), либо может быть свободно откачен с уверенностью, что откатилось в точности одно логическое изменения. Это бесценное свойство при бинарном поиске ошибок. Более того, очевидно, что логичные изменения гораздо проще отсматривать в процессе код-ревью.»
            • 0
              бывают ситуации, когда переименование файла и изменения в нём же — это одно логическое изменение
              • 0
                если я правильно помню, на этом глючит svnlog. если не так, то нужно исправить текст.
  • 0
    >>Вопросы, пожелания, предложения?

    Поясните хотя бы вначале сленговые слова, для того, чтобы непосвященным было понятнее раз, для того, чтобы выглядело по-человечески, а не перепиской в аське два. =)

    А использование шахматных терминов порадовало :).

    Ну и конечно, у меня сильное ощущение, что текст можно было бы сократить минимум в два раза, если бы поперечитывали и подредактировали.
    • 0
      это вобщем-то не сленговые слова, а команды и понятия из Subversion. Ежели вы пользуетесь SVN-ом, вам эти слова знакомы, а если вы только собираетесь пользоваться, то можно/нужно почитать что-то другое, перед этой статьей.
      http://google.com/search?hl=ru&q=%D1%87%…
  • 0
    >> Мы молчим про пустые журнальные сообщения — это вообще худшее что может быть. Это допустимо только для совершенно далеких от программирования сотрудников (типа дизайнеров), которые на этот ваш сабвершен с трудом вообще согласились.

    На эту тему удивил лог acts_as_state_machine, в котором вообще почти все коменты к коммитам пустые: http://codenotifier.com/projects/15
    Мож у чувака какой-то стоит ловкий просмотрщик лога, который диффы сразу кажет - при том объеме кода комменты может и излишни.
  • +1
    По поводу хранения кусков кода в закоментированном состоянии есть контр пример. Часто бывает, что какая-то функциональность не вынесена в отдельную структурную единицу (функцию или т.п.), а исключить её из основного кода надо. При этом точно известно, что она понадобится в будующем (ну или возможно понадобится). Если эту часть кода удалить, считая что в репозитории она всёравно есть, то возникнут две проблемы, когда её надо будет вернуть:
    1. никто не вспомнит в какой ревизии оно отсюда исчезло (по diff-ам и blame-ам можно очдолго ползать, если уже много накоммитили)
    2. даже если удастся достать этот кусок из репозитория, он может быть уже протухшим (изменились имена глобальных переменных и прочие последствия глобальных рефакторингов)

    Поэтому в своей практике я вполне допускаю закоментированные куски кода, при условии, что "глобальные рефакторинги" на них распространяются. Да, это не гарантирует работоспособность (всётаки код не исполняется и никак проверить нельзя), но при достаточной аккуратности даёт больше чем удаление и надежда на репозиторий.
    • 0
      Гм. А как рефакторинги могут распространяться на закомментированые куски кода? Я думал, sed-ом уже никто не рефакторит :). Или какие-то IDE-шки лазят в комментарии?
      • 0
        я может вас огорчу, но в мире существуют языки для которых автоматический рефакторинг не настолько хорош, как хотелось бы
        когда пишешь на них приходится рефакторить "sed-ом", а иногда... вручную
        • 0
          Наверное, это... php? :)
          Впрочем, понятно. Дествительно. Просто ява сильно расслабляет в этом плане :).
  • 0
    Зря вы так про дизайнеров, не так там всё запущено. Другое дело, что часто правки дизайнерские описывать дольше, чем делать. в этом случае мы просто указываем, что было поправлено и указываем ID таска, где задача расписана более подробно, зачастую со скринами того, что надо сделать.
  • 0
    Какая-то СС'овская терминология у Вас :)) А вообще все можно упростить если на полную использовать merge.
    • 0
      что именно можно упростить, если на полную использовать мердж? и определите понятие "на полную". и скажите, в какой системе.
      • 0
        Проще сказать как делаем мы (как я понимаю, это классический подход). Система - subversion 1.5 (beta, с поддержкой merge tracking).
        1) есть транк (основная ветка), куда делаюттся аккуратные, протестированные, логически целостные и задокумментированные коммиты. Сюда также коммитятся все багфиксы.
        2) Для каждой мажорной фичи или версии создается бранч, куда можно коммитить грязно и беспорядочно, для ускорения процесса. Здесь допускаются битые ревизии. 3) Есть branches/stable, куда после тестирования тестером, перед рилизом, мерджатся все изменения в транке. 4) Бранчи мерджатся предварительно в транк.
        "На полную" - как раз, используя svn 1.5/(1.4.99). У collabnet есть блог (ссылки не могу прилагать), где хорошо описываются недостатки ручного мерджа (да это просто ад), и то как он будет работать в 1.5.
        • 0
          Ну вы герои, использовать нерелизнутый 1.5, который судя по тракеру ещё достаточно сырой. Что ещё тут можно сказать.
          • 0
            Пока что ни одного нарицания, ни к серверу(кроме необходимости переконвертить репозитории из BDB в FSFS (BDB не поддерживается, имейте ввиду когда будете создавать новый репо)), ни к соответствующему TortoiseSVN (много хороших фичей помимо мерджинга, включая поддержку висты)
        • 0
          Бранч для каждой мажорной фичи? Не моноговато ли? Скажите, сколько у вас на текущий момент бранчей?
          • 0
            мажорная фича - пожалуй неверно сказано. Речь идет о переработке коры, любой рефакторинг и тд, все что не подлежит постоянному коммиту в транк и не является багфиксом. В данный момент всего одна, над которой идет работа. В новом TortoiseSVN, кстати, мердж минорных изменений в параллельной ветке (trunk->stable) отличается от мерджа из форкнутой(фичевой) ветки.
  • 0
    Если бы понял что-то, то наверно было бы очень интересно )))
  • +1
    Спасибо, хорошая статья. От себя добавлю, что полезно в лог коммита писать номер таска в баг(/таск)трекере.

    И наверное вам надо было в начале статьи поставить ссылку на какую-нибудь вводную статью "Что такое контроль версий и с чем его едят" :)
  • 0
    Не понятно, почему вы не написали, что trac с subversion особенно хорош с trac'овским hook'ом на post-commit. В этом случае появляется возможность закрывать задачи и писать в комментарии к задачам с помощью log message.
  • 0
    А не помнит ли кто-нибудь линк на статью про git на хабре? Вообще было бы интересно почитать нечто подобное про git, ибо есть нюансы.

    И еще автору: мне кажется было бы мега полезно привести примеры правильных и неправильных коммитов (особенно description-ов).
    • 0
      Ага Git интересная штука. В некоторых случаях (когда доходит до branch-merge) удобнее чем Subversion.
      • НЛО прилетело и опубликовало эту надпись здесь
  • 0
    Cпасибо за статью, воспринимается легко и познавательно. По поводу предложений, хотелось бы в таком же стиле статью про управление ветками (branches), про то как правильно создавать, коммитить, обьединять и разрешать конфликты.
  • +1
    Насчёт хорошей документируемости коммитов.

    Нельзя не упомянуть, как в контексте исследования старого (legacy) кода,
    так и в качестве наказаний для молчунов (пустые логи) и лентяев (лог просто "fixed") о возможности править svn:log:

    svn pe svn:log -r XXX --revprop

    В первом случае позволяет удобным образом хранить замечания о давних (и невнятных) коммитах, во втором... заменить битьё по рукам полезным деянием.
  • 0
    Большое спасибо. Жму руку, тащу в корпоративную вики (копирайты сохраняю, конечно :).
    • 0
      Корпоративная вики, а вы случайно Trac не пользуетесь? Там есть встроенная вики. Мы на данный момент используем её.
      Или у вас какой-то другой движок?
      • 0
        Trac не пользуемся. Используем MoinMoin, http://www.google.ru/search?q=moinmoin, рекомендую от всей души. Но в виках главное — вовсе не движок, главное — контент, а движок — дело вкуса и удобства. :)
  • 0
    особо нового ничего для себя не вынес, но спасибо!

    зы особенно хорошо с результатами коммитов IJ Idea умеет работать - сама проверит обновления, покажет чейджлог, предупредит перед началом изменения файла, что он уже устарел.
  • 0
    спасибо, полезная заметка. еще чуть упорядочить текст и немного примеров, поясняющих каждый абзац и получится отличная статья.
  • 0
    Некоторые замечания по названиям фаз разработки (дебют, миттельшпиль и эндшпиль)
    Мне как-то привычнее общепринятые термины feature-freeze, code-freeze, system-test-freeze,
    на которых всё меньше вносится крупных изменений в код и добиваются большей стабильности, всё жестче ограничения на коммиты.

Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

Интересные публикации