Pull to refresh

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

Reading time 4 min
Views 18K
Original author: David Lloyd
Я постоянно делаю коммиты в проекты 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. Начните разговор


Если после всего этого вам всё ещё что-то не нравится, но вы не уверены, почему, возможно, придётся просто смириться. Или сказать: «Мне это не нравится, но я не уверен, почему, можем поговорить об этом?» Это разумный вопрос, и даже хотя это может занять немного времени, но часто стоит того, потому что теперь у вас два человека, и оба учатся (объяснять и слушать), а не два человека, которые противостоят друг другу.

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

Резюме


Избегайте гиперболических или напыщенных утверждений, избегайте споров, элитарных или унижающих выражений и конструкций вроде «очевидно» и «почему бы вам просто...». Используйте чёткие, фактические утверждения и поддерживающие формулировки, задавайте вопросы и продвигайтесь вперёд. Помните, что коллеги и контрибуторы — тоже люди. Их время заслуживает такого же уважения, как и ваше.
Tags:
Hubs:
+40
Comments 35
Comments Comments 35

Articles