10 советов, как ревьюить код, который вам не нравится

в 20:23, , рубрики: open source, психология, управление проектами

Я постоянно делаю коммиты в проекты 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

Источник

* - обязательные к заполнению поля