Google: Writing good CL descriptions

Google, основываясь на многолетнем опыте, представил свои стандарты того, как лучше всего проводить code review. Все вместе они представляют собой один законченный документ, разбитый на множество отдельных разделов. Это перевод стандартов по проведению Code Review от Google. Вот ссылка на оригинал!

Терминология:

CL — расшифровывается как «список изменений» (“ChangeList”), что означает одно автономное изменение, которое было отправлено в систему управления версиями или подвергается проверке кода. Другие организации часто называют это «изменением» или «патчем».

Пишем правильное описание для CL

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

Будущие разработчики будут искать ваш CL на основе его описания. И если вся важная информация содержится в коде, а не в описании, им будет намного сложнее найти ваш CL.

Заголовок CL

• Краткое описание того, что делается.
• Полное предложение в виде распоряжения.
• Пустая строка после заголовка.

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

По традиции, заголовок CL представляет собой законченное предложение. Например, «Удалить RPC FizzBuzz и заменить его новой системой» вместо «Удаление RPC FizzBuzz и замена его новой системой».

Информативное тело описания

Остальная часть описания должна быть более информативной. Сюда может входить краткое описание решаемой проблемы и пояснение, почему это подход лучше. Если есть какие-либо недостатки в подходе, их следует упомянуть. При необходимости включите дополнительную информацию, такую как номера ошибок, результаты тестов и ссылки на проектные документы.

Даже небольшие CL заслуживают должного внимания к деталям. Поместите CL в контекст.

Плохое описание CL

• «Fix bug» — неадекватное описание CL. Какая тут ошибка? Здесь неясно, что предпринял разработчик, чтобы исправить этот баг. Вот еще примеры плохого описания CL:
• «Fix build.».
• «Add patch.»
• «Moving code from A to B.»
• “Phase 1.”
• «Add convenience functions.»
• «Kill weird URLs.»

Некоторые из них являются реальными описаниями CL. Их авторы могут полагать, что они предоставляют полезную информацию, однако они никак не описывают CL.

Хорошее описание CL

Вот несколько примеров хороших описаний.

Изменение функциональности

Пример: “Commit: remove size limit on RPC server message freelist.

Servers like FizzBuzz have very large messages and would benefit from reuse. Make the freelist larger, and add a goroutine that frees the freelist entries slowly over time, so that idle servers eventually release all freelist entries.”

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

Рефакторинг

Пример: “Commit: Construct a Task with a TimeKeeper to use its TimeStr and Now methods.

Add a Now method to Task, so the borglet() getter method can be removed (which was only used by OOMCandidate to call borglet’s Now method). This replaces the methods on Borglet that delegate to a TimeKeeper.

Allowing Tasks to supply Now is a step toward eliminating the dependency on Borglet. Eventually, collaborators that depend on getting Now from the Task should be changed to use a TimeKeeper directly, but this has been an accommodation to refactoring in small steps.

Continuing the long-range goal of refactoring the Borglet Hierarchy.”

Первая строка описывает, что выполняет CL и что отличает эти изменения от предыдущего варианта. В остальной части описания говорится о конкретной реализации, контексте CL, о том, что решение не является идеальным, и о возможном будущем направлении. И также объясняется, почему происходит это изменение.

Небольшой CL, который нуждается в контексте

Пример: “Commit: Create a Python3 build rule for status.py.

This allows consumers who are already using this as in Python3 to depend on a rule that is next to the original status build rule instead of somewhere in their own tree. It encourages new consumers to use Python3 if they can, instead of Python2, and significantly simplifies some automated build file refactoring tools being worked on currently.”

Первое предложение описывает, что на самом деле выполняется. Остальная часть описания объясняет, почему вносятся изменения, и дает ревьюеру довольно подробную информацию о контексте.

Проверьте описание CL перед коммитом

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