| 1 | выдержки из перевода статьи [http://habrahabr.ru/blogs/code_review/64086/ Я ненавижу тебя: твой код – отстой!] |
| 2 | Оригинал [http://mumak.net/stuff/your-code-sucks.html Your Code Sucks and I Hate You: The Social Dynamics of Code Reviews] |
| 3 | |
| 4 | |
| 5 | == Что такое плохо == |
| 6 | |
| 7 | === Ad hominem (переход на личности – прим. перев.) === |
| 8 | |
| 9 | Здесь практически нечего добавить: инспектируйте программу, а не программиста. Замечания о его личности лишь усложняют восприятие им критики. |
| 10 | Совет: если вы пишите негативные комментарии, пишите «патч» или «код» вместо «ты». Например, вместо «У тебя глюк в get_message» пишите «После применения этого патча в get_message появился глюк». |
| 11 | |
| 12 | === Туманные результаты ревизии === |
| 13 | |
| 14 | Если всё, что вы можете сказать, это «В этом патче появился глюк в get_message», то вы плохой рецензент. Цель ревизии – улучшить код, а не загадывать автору загадки. |
| 15 | |
| 16 | Автор должен иметь возможность однозначно выделить и понять каждую отдельную мысль в вашем замечании, а также всё замечание целиком. Обзоры с нечёткими результатами превращаются в бесконечные дискуссии о коде, которые скорее служат тому, чтобы доставить удовольствие рецензенту, чем повешению качества кода. |
| 17 | |
| 18 | === Не путайте личные предпочтения с объективными достоинствами === |
| 19 | |
| 20 | Это проблема обозревателей в любой области: кинокритика, литературного редактора, научного рецензента. Не обязательно на самом деле хорошо то, что нравится мне. И уж тем более не обязательно на самом деле плохо то, что не нравится мне. Один из аспектов становления выдающегося программиста это моделирование собственных предпочтений в соответствии с реальными, объективными достоинствами. |
| 21 | |
| 22 | Если вы рецензируете патч, который прекрасно устраняет какую-либо проблему, используя парадигму императивного программирования, то не стоит критиковать его лишь за то, что он не следует функциональной парадигме. Так вы дискредитируете идею ревизий, превращаю её в игру по угадыванию предпочтений рецензента, а она должна способствовать создания действительно хорошего код. |
| 23 | |
| 24 | Формулирование комментариев в виде вопросов поможет проверяющему избежать такой оплошности, например: «Не думали ли вы использовать функциональный стиль?», «Почему вы не используете регулярные выражения для решения этой задачи?» и т.д. |
| 25 | |
| 26 | См. также «Чёткий ответ – хороший ответ». |
| 27 | |
| 28 | === «Пока вы ещё здесь…» === |
| 29 | |
| 30 | При обзоре кода всегда есть соблазн попросить автора исправить заодно и какие-нибудь другие проблемы. Такой подход может быстро перерасти в полномасштабную операцию по наведению глянца на большом участке кода, хотя изначально автор и проверяющий должны были утвердить лишь небольшое улучшение. |
| 31 | |
| 32 | Лучший совет в такой ситуации: дорожить поэтапным улучшением. См. «Лучшее враг хорошего». |
| 33 | |
| 34 | === «Флибустьерство» === |
| 35 | |
| 36 | «Флибустьерство» (filibustering) это американский политический термин, обозначающий затягивание дебатов по законопроекту с целью помешать его принятию. Иногда это делается и в проектах свободного ПО с целью саботировать принятие определённого патча. |
| 37 | |
| 38 | «Флибустьерство» также может возникать непреднамеренно. Человек, проводящий ревизию, не принимает патч по причине отсутствия блочных тестов, автор спрашивает, как ему написать блочные тесты для этого кода, а рецензент так и не удосуживается ответить. А ведь омимо этих неприятностей, у автора есть ещё и свои дела, мешающие ему продолжать присылать новые патчи. |
| 39 | |
| 40 | См. «Туманные итоги обзора» и «Быстрый ответ – хороший ответ». |
| 41 | |
| 42 | == Что такое хорошо == |
| 43 | |
| 44 | === Чёткий ответ – хороший ответ === |
| 45 | |
| 46 | Указывайте конкретные строки кода, которые можно улучшить. Опишите, что с в них не так. Предложите способ сделать лучше. Помогите автору определить пределы совершенствования. |
| 47 | |
| 48 | === Быстрый ответ – хороший ответ === |
| 49 | |
| 50 | Как только автор представил свои патчи на ревизию, он больше не может над ними работать. Он должен дождаться ответа ревизора, прежде чем продолжить. Пишите свои замечания в обзор как можно скорее, чтобы внимание автора не успело рассеяться. |
| 51 | |
| 52 | === Лучшее враг хорошего === |
| 53 | |
| 54 | Нельзя сделать идеальный патч, который бы разом решил все ваши проблемы. Не гонитесь за идеалом, работайте над поэтапным улучшением. |
| 55 | |
| 56 | === Будьте благодарны === |
| 57 | |
| 58 | Кто-то попытался улучшить ваш продукт. Они потратили свои силы и фантазию на то, чтобы помочь вам, а теперь представили свой труд на суд общественности: так поблагодарите их. |
| 59 | |
| 60 | В проекте Divmod практикуют принцип включения в каждый обзор кода хотя бы одной похвалы. Всегда можно найти, за что похвалить автора, пусть даже лишь за то, что он взялся поработать над этой частью кода. |
| 61 | |
| 62 | === Задавайте вопросы === |
| 63 | |
| 64 | Рецензент должен задавать вопросы. Нельзя ошибиться, задавая вопросы. В худшем случае автор даст тривиальный ответ. В лучшем случае оба, и автор, и проверяющий, откроют для себя что-то новое. |
| 65 | |
| 66 | == Заключение == |
| 67 | |
| 68 | Ревизии кода дают человеку уникальную возможность расти как программисту, при этом улучшая продукт, который он создаёт. Каждый проект может выбирать из многих вариантов проведения ревизий и предоставляемых ими возможностей. Тщательное обдумывание технологий и процессов взаимодействия людей при проведении обзоров позволит проектам с открытым кодом избежать сложностей, которые отпугивают новичков и утомляют давнишних участников, и сосредоточиться на разработке наилучшего программного обеспечения. |