выдержки из перевода статьи ​Я ненавижу тебя: твой код – отстой! Оригинал ​Your Code Sucks and I Hate You: The Social Dynamics of Code Reviews

Что такое плохо

Ad hominem (переход на личности – прим. перев.)

Здесь практически нечего добавить: инспектируйте программу, а не программиста. Замечания о его личности лишь усложняют восприятие им критики. Совет: если вы пишите негативные комментарии, пишите «патч» или «код» вместо «ты». Например, вместо «У тебя глюк в get_message» пишите «После применения этого патча в get_message появился глюк».

Туманные результаты ревизии

Если всё, что вы можете сказать, это «В этом патче появился глюк в get_message», то вы плохой рецензент. Цель ревизии – улучшить код, а не загадывать автору загадки.

Автор должен иметь возможность однозначно выделить и понять каждую отдельную мысль в вашем замечании, а также всё замечание целиком. Обзоры с нечёткими результатами превращаются в бесконечные дискуссии о коде, которые скорее служат тому, чтобы доставить удовольствие рецензенту, чем повешению качества кода.

Не путайте личные предпочтения с объективными достоинствами

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

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

Формулирование комментариев в виде вопросов поможет проверяющему избежать такой оплошности, например: «Не думали ли вы использовать функциональный стиль?», «Почему вы не используете регулярные выражения для решения этой задачи?» и т.д.

См. также «Чёткий ответ – хороший ответ».

«Пока вы ещё здесь…»

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

Лучший совет в такой ситуации: дорожить поэтапным улучшением. См. «Лучшее враг хорошего».

«Флибустьерство»

«Флибустьерство» (filibustering) это американский политический термин, обозначающий затягивание дебатов по законопроекту с целью помешать его принятию. Иногда это делается и в проектах свободного ПО с целью саботировать принятие определённого патча.

«Флибустьерство» также может возникать непреднамеренно. Человек, проводящий ревизию, не принимает патч по причине отсутствия блочных тестов, автор спрашивает, как ему написать блочные тесты для этого кода, а рецензент так и не удосуживается ответить. А ведь омимо этих неприятностей, у автора есть ещё и свои дела, мешающие ему продолжать присылать новые патчи.

См. «Туманные итоги обзора» и «Быстрый ответ – хороший ответ».

Что такое хорошо

Чёткий ответ – хороший ответ

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

Быстрый ответ – хороший ответ

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

Лучшее враг хорошего

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

Будьте благодарны

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

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

Задавайте вопросы

Рецензент должен задавать вопросы. Нельзя ошибиться, задавая вопросы. В худшем случае автор даст тривиальный ответ. В лучшем случае оба, и автор, и проверяющий, откроют для себя что-то новое.

Заключение

Ревизии кода дают человеку уникальную возможность расти как программисту, при этом улучшая продукт, который он создаёт. Каждый проект может выбирать из многих вариантов проведения ревизий и предоставляемых ими возможностей. Тщательное обдумывание технологий и процессов взаимодействия людей при проведении обзоров позволит проектам с открытым кодом избежать сложностей, которые отпугивают новичков и утомляют давнишних участников, и сосредоточиться на разработке наилучшего программного обеспечения.