7 December 2015

Перевод: Инструкция по проведению code review

Voximplant corporate blogWebsite developmentProgrammingVersion control systems
Original author: Mike Burns
Не так давно мой коллега переводил интересную статью о code review, перевод хабражителям понравился. А сегодня утром запутанный граф кроссылок вывел eyeofhell на еще более крутую статью. Вашему вниманию предлагается перевод краткой, но емкой инструкции о том, как делать review чужого кода и пережить review собственного. В отличие от упомянутой выше статьи, эта больше фокусируется на практических аспектах code review и содержит множество полезных рекомендаций как и что делать, чтобы не было мучительно больно. Хинт: чтобы почитать оригинал, кликните на имени автора в плашке под переводом.



Для всех участников code review



  • Примите как данность, что многие архитектурные решения — это вопрос личного вкуса разработчика. Старайтесь обсуждать только сильные/слабые стороны и как можно быстрее принять решение что делать с кодом.
  • Не требуйте, а задавайте вопросы. Например: «Что ты думаешь по поводу использования имени идентификатора :user_id?». Примечание переводчика: этот совет основан на наблюдениях, что человеческий мозг склонен выдавать сильную эмоциональную реакцию на критику и требования, в то время как приглашение к обсуждению часто проходит мимо эмоциональной составляющей и позволяет спокойно общаться о коде.
  • Не стесняйтесь уточнять непонятные моменты. Например: «Не могу понять, что тут происходит. Можешь чуть подробнее объяснить?».
  • Старайтесь избегать разделение кода на свой/чужой. С подозрением относитесь к словам «мой», «не мой», «твой».
  • Обсуждайте программу, а не программиста. Старайтесь избегать перехода на личности и не используйте выражения вроде «тупое решение» или «идиотский код». Проводите code review так, как будет все члены команды — умные, интеллигентные люди с благими намерениями :)
  • Старайтесь явным образом выражать свои мысли. Люди не всегда способы понять ваши намерения online.
  • Старайтесь быть скромнее, например: «не уверен что это будет работать, давай проверим?».
  • Не гиперболизируйте и по возможности избегайте таких слов как «всегда», «никогда», «ничего» итд.
  • Сарказм тоже не очень хорошая штука.
  • Старайтесь быть собой, как бы заезжено не звучала эта фраза. Если emoji, анимирнованные gif'ы и юмор это «не ваше» — то не стоит себя насиловать, стараясь быть похожим на других членов команды.
  • Если в обсуждении встречается слишком много заметок «я не понял» и «есть другой путь решения этой задачи», то хорошей идеей будет обсудить этот код лично, а затем записать краткий follow up в рамках code review.


Если ваш код подвергся code review



  • Хорошим тоном считается поблагодарить ревьювера за проделанную работу, например: «спасибо что нашли эту интересную ситуацию, мы поменяем логику завершения работы».
  • Старайтесь не принимать результаты code review близко к сердцу. Обсуждают код, а не вас.
  • В спорных ситуациях старайтесь объяснить, с какой целью был написан этот код. Ответ на вопрос «зачем?» упрощает коммуникации.
  • Хорошей идеей будет вынести объемные исправления в отдельные задачи, а не складывать все в одно место.
  • Сделайте ссылку на code review из соответствующей задачи, например: «готово, можно ревьювить: github.com/organization/project/pull/1».
  • Не объединяйте коммиты (squash), пока ревью не закончено. У ревьюверов должна быть возможность посмотреть на индивидуальные коммиты, которые исправляют отдельные вопросы по коду.
  • Попробуйте понять точку зрения ревьювера.
  • Хорошим тоном считается ответить на каждый комментарий.
  • Не нужно делать merge, пока не пройдены все тесты continuous integration.
  • Merge рекомендуется делать только если вы уверены в коде и его влиянии на проект.


Если вы хотите подвергнуть code review чужой код



В первую очередь постарайтесь разобраться зачем был написан этот код. Это багфикс? Новая фича? Рефакторинг? Кровавое месиво из всего вышеперечисленного? Затем:

  • Старайтесь явным образом указывать в каких замечаниях вы полностью уверены, а в каких — не очень.
  • Подсказывайте способы упростить код без потери функциональности.
  • Если обсуждение кода уходит в философские или академические дебри, то такое обсуждение хорошо вынести в оффлайн. Например, на традиционные пятничные посиделки. В вашей команде же есть пятничные посиделки?
  • Предлагайте альтернативные решения, но исходите из предположения что автор уже их попробовал. Например: «что думаешь по поводу кастомного валидатора?»
  • Попытайтесь понять точку зрения автора кода.
  • В конце code review пометьте pull request комментарием «можно мерджить».


Замечания по оформлению кода



Если вы видите, что код явным образом нарушает принятый в команде стиль кодирования — нелишним будет указать на это во время code review. Если окажется, что кто-то в команде не согласен с таким стилем, то обсуждение рекомендуется выносить в отдельный тикет.
Tags:code reviewvoximplant
Hubs: Voximplant corporate blog Website development Programming Version control systems
+53
35.2k 277
Comments 19
Top of the last 24 hours