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