Хабрахабр

Часть вторая. Как проходить code review по версии Google

Возможно вы читали первую часть статьи про код ревью со стороны ревьювера (кстати, мы уже успели ее обсудить в последнем выпуске подкаста "Цинковый прод").

Так как статья набрала много лайков, пишу обещанное продолжение про код ревью с другой стороны — со стороны автора изменений кода

Как обычно, будем говорить MR (Merge Request) вместо CL, потому что термин CL мало кто понимает.

Оригинал инструкции для авторов MR по версии Google можно посмотреть здесь, а я дам краткую выжимку.

Итак, поехали

Описание MR

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

Причем по традиции применяется императивный (повелительный) стиль, т.е. В первой строчке (в заголовке) должно быть одной фразой описано, что было сделано в MR. Delete blablabla, а не Deleting blablabla

Даже в маленьком MR что-то такое должно быть. Само описание должно быть информативным, содержать краткое описание решаемой проблемы, ссылки на необходимые документы (если необходимо), задачи таск трекера и другой контекст.

Описание типа "Fix bug", понятное дело, считается неадекватным.

MR должен быть как можно меньше

  • Маленький MR можно быстро проверить
  • Проверка будет более осмысленной
  • Меньше вероятность упустить баг
  • Не так обидно, если весь MR будет отклонен. Ведь очень плохо, когда сделана большая работа, а потом выясняется, что все это вообще было не нужно
  • Проще вливать изменения, меньше конфликтов
  • Легче добиться хорошего качества кода
  • Чем больше изменений за раз, тем сложнее откатывать код при такой необходимости

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

100 строк кода — это уже большой или еще нет? Конечно, не может быть жесткого правила, какой MR будет считаться большим, какой маленьким. Кто знает.

  • MR должен делать что-то одно. Обычно это не вся фича, а ее часть
  • Выделяйте рефакторинг в отдельный MR

MR должен быть маленьким, но самодостаточным

  • Всё, что необходимо ревьюверу для понимания MR, должно быть в этом MR
  • После вливания кода, система должна функционировать нормально.
  • Если добавляется метод API, в этом же MR должны быть продемонстрированы и способы его использования. Другими словами, MR не должен быть настолько маленьким, чтобы трудо было понять как он будет использоваться, к каким последствиям приведет.
  • Покрывайте код тестами, причем тесты должны быть в этом же MR

Не принимайте близко к сердцу

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

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

Если и это не помогло, Гугл советует обратиться к менеджеру.

Текст скрывает половину эмоций; например, сказанное в шутливо-дружественном тоне в текстовом виде может казаться жестким наездом. Из моего опыта хочу добавить, что зачастую человек пишущий невежливый комментарий, часто не отдает отчет, что это может смотреться как-то агрессивно. Поэтому полностью согласен с гуглом — в случае недопонимания общаться только лично!

Объясняйте кодом

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

Реакция на комментарии ревьювера

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

Однако если ревьювер все же не прав, смело пишите об этом, снабдив ответ аргументами и фактами.

Выводы

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

А неизбежно возникаемые разногласия решать, приходя к консенсусу в вежливой профессиональной форме.

В следующем выпуске "Цинкового прода" мы обязательно обсудим эту статью (и многое другое), поэтому не забудьте подписаться на наш подкаст, иначе пропустите самое интересное!

Теги
Показать больше

Похожие статьи

Добавить комментарий

Ваш e-mail не будет опубликован. Обязательные поля помечены *

Кнопка «Наверх»
Закрыть