Я постоянно делаю коммиты в проекты open source (Red Hat и др.). И заметил, что больше всего времени отнимают негативные код-ревью, субъективные по сути. Чаще всего такое происходит с коммитами, где мейнтейнеру по какой-то причине не нравится ваше изменение. В лучшем случае такая стратегия код-ревью приводит к потере времени в бессмысленных спорах; в худшем случае он активно препятствует коммиту, создавая враждебную и элитарную среду.
Код-ревью должен быть объективным, кратким и, по возможности, содержать только определённые факты. Это не политический или эмоциональный спор, а технический. Его цель — продвижение вперёд, развитие проекта и всех участников. Любой коммит должен оцениваться только по существу, а не по субъективному мнению.
Стратегии код-ревью
Вот несколько стратегий, которые следует иметь в виду при рассмотрении кода, которые вам (как мейнтейнеру) по какой-то причине не нравится:
1. Перефразируйте возражение в виде вопроса
- Плохо: «Это изменение сделает XXX невозможным» (Это преувеличение; неужели на самом деле это невозможно?)
- Хорошо: «Как мы будем делать XXX после вашего изменения?»
2. Избегайте преувеличений
Просто высказывайте свои опасения и задавайте вопросы, чтобы помочь достичь желаемого результата.
- Плохо: «Это изменение уничтожит производительность».
- Хорошо: «Похоже, что X может быть медленнее, чем существующий Y; вы проводили измерения/собрали данные, чтобы показать, что это не так?»
- Лучше (если у вас есть время): «Я пока собираю данные. Попробую проверить, что X не медленнее Y».
- Тоже хорошо: «Это изменение изменяет одиночный цикл O(n) на дважды вложенный цикл O(n²); не повлияет ли это на производительность?»
3. Оставьте ехидные комментарии при себе
Некоторые мысли лучше держать при себе. Если вы не можете сказать вежливо, лучше промолчите.
- Плохо: «Думаю, что это плохое изменение, которое всё порушит».
- Плохо: «Вы уверены, что разработка программного обеспечения для вас правильный карьерный выбор?»
4. Действуйте позитивно
Может, у вас было другое представление о том, как решить проблему? Если вы действуете позитивно, то в конечном итоге найдёте решение, которое лучше, чем любой из исходных вариантов.
- Плохо: «Это изменение отстой, моя версия лучше».
- Хорошо: «У меня тоже есть изменение для этого места: возможно, мы можем сравнить и/или объединить идеи».
- Тоже хорошо «У меня в работе аналогичное изменение, но я решил сделать X, потому что ZZZ; почему вы выбрали Y?»
5. Помните, что у всех разный опыт
Во всех остальных отношениях абсолютно компетентный инженер может годами не знать некоторых фактов, которые вы принимаете за здравый смысл. Нет ничего зазорного в том, чтобы сказать очевидную вещь, если только вы не покровительствуете или ехидствуете.
- Плохо: «Разве вы не видите, что это явно неправильно?»
- Хорошо: «Это неверно, потому что вызывает исключение нулевого указателя, когда X равен Y».
6. Не преуменьшайте сложность того, что не очевидно
Помните, что очевидные для вас вещи могут быть очевидны не для всех. Предлагая альтернативные подходы и указывая на полезные примеры, вы поможете всем синхронизироваться.
- Плохо: «Почему бы просто не фробнуть гноззл?»
- Хорошо: «Если фробнуть гноззл, то данная часть может стать проще (см. XXX для примера)».
7. Относитесь с уважением
Иногда коммит просто не соответствует минимальному стандарту качества. Вполне нормально сказать об этом, но проявить уважение не требует дополнительных усилий.
- Плохо: «Это глупый код, написанный глупым человеком».
- Хорошо: «Спасибо за ваш вклад. Однако он не может быть принят в нынешнем виде; существует множество проблем (как указано выше)».
- Тоже хорошо: «Как указано выше, с этим коммитом есть несколько проблем. Может, отступить на шаг и поговорить о сценариях использования? Это поможет нащупать путь».
8. Управляйте ожиданиями (и своим временем)
Если коммит слишком большой и его нельзя быстро рассмотреть, то нормально сразу об этом сказать. Затем ищите вариант решения.
- Плохо: «Я не принимаю его, он слишком большой».
- Тоже плохо: игнорировать коммит, пока он не исчезнет.
- Хорошо: «Не могли бы вы разбить его на более мелкие коммиты? У меня не слишком много времени для код-ревью, а это просто слишком большой/сложный коммит для одного ревью».
9. Скажите «пожалуйста»
Просто сказав «Пожалуйста», вы показываете, что уважаете время отправителя, особенно когда хотите изменить форматирование или стиль, что может показаться незначительным изменением. Примеры:
- «Не могли бы вы оформить изменения в пробелах в другом пул-реквесте?»
- «Не могли бы вы выровнять эти определения переменных, чтобы их было легче читать?»
10. Начните разговор
Если после всего этого вам всё ещё что-то не нравится, но вы не уверены, почему, возможно, придётся просто смириться. Или сказать: «Мне это не нравится, но я не уверен, почему, можем поговорить об этом?» Это разумный вопрос, и даже хотя это может занять немного времени, но часто стоит того, потому что теперь у вас два человека, и оба учатся (объяснять и слушать), а не два человека, которые противостоят друг другу.
Даже квалифицированные и опытные инженеры должны быть в состоянии сказать: «Я не понимаю, почему мне это не нравится». Это не приглашение атаковать позицию рецензента, а скорее честный поиск знаний.
Резюме
Избегайте гиперболических или напыщенных утверждений, избегайте споров, элитарных или унижающих выражений и конструкций вроде «очевидно» и «почему бы вам просто...». Используйте чёткие, фактические утверждения и поддерживающие формулировки, задавайте вопросы и продвигайтесь вперёд. Помните, что коллеги и контрибуторы — тоже люди. Их время заслуживает такого же уважения, как и ваше.
Автор: m1rko
