[Перевод] Перевод: Инструкция по проведению code review07.12.2015 09:33

Для всех участников 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. Если окажется, что кто-то в команде не согласен с таким стилем, то обсуждение рекомендуется выносить в отдельный тикет.

• ↑

  —

  ↓

• 204

• Добавить в избранное 4