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