Ревью на ясность и тон — Примеры

Пример 1 — Ревью, перечисляющее двадцать равных наблюдений

Комментарии ниже — то, что ревьюер оставил на небольшом изменении; скажите, что автор может из этого исполнить, затем перепишите их так, чтобы приоритеты и места были ясны.

- плохое имя
- надо добавить тесты
- запутанно
- я бы использовал map вместо вложенного цикла
- опечатка в docstring
- обрабатывает ли это null?

Решение

блокер: строка 42 — `debit` возвращает "pending", когда все повторы
провалились, поэтому вызывающий считает неудавшееся списание оплаченным.
Что должен увидеть вызывающий?

блокер: строки 30-38 — ни один тест не покрывает путь повторов, поэтому
откат всё равно прошёл бы набор. Можно добавить тест, падающий без него?

предложение: `data` хранит разобранную запись, а не сырой ответ — более
ясное имя сделало бы строку 12 читаемой.

придирка: опечатка в docstring на строке 8.
убрано: map — это вкус, null исключён типом, а цикл идёт по списку из трёх.

Переписывание — в основном удаление: половина комментариев не изменила бы день автора, а без меток важности автору приходится угадывать, что важно. Каждый комментарий, который уцелел, называет строку и формулирует ожидание и наблюдение, поэтому на него можно ответить доказательством. Ловушка в том, что длинный список кажется основательным, — это самый частый способ упустить настоящий дефект.

Пример 2 — Один дифф, делающий две вещи

Изменение приходит одним диффом на 620 строк: переименование модуля по одиннадцати файлам плюс правка правила округления в двух из них. Скажите, что ревьюер реально может сделать, затем напишите ответ.

дифф: 620 строк, 13 файлов
  файлы 1-11: переименовать модуль цены и обновить импорты
  файлы 12-13: сменить округление с floor на nearest плюс его тест
приписка автора: "суть в исправлении округления, переименование попутно"

Решение

предложение: разделите это до ревью. Одним диффом его просмотрят мельком,
а 618 строк переименования прячут две строки, меняющие поведение.

  PR 1 — только переименование, без смены поведения: механическое и
  быстро проверяемое
  PR 2 — смена округления плюс тест, падающий на старом коде

Большой дифф получает поверхностное ревью, что бы ревьюер ни намеревался, а 618 строк переименования хоронят две строки, меняющие поведение. Разделение поднимает внимание к смене поведения и заставляет позднейший bisect попасть на осмысленный коммит. Ловушка — приписка автора: указание на важные строки кажется решением, но ревьюеру всё равно надо держать в поле зрения 620 строк, чтобы этому доверять.