Дифф на 1240 строк я читал два часа. Нашёл три расхождения с исходным поведением, из них одно серьёзное. Уверен, что нашёл не всё — на восьмисотой строке внимание кончается, а дальше начинается вежливое пролистывание.
Проблема даже не в объёме. Проблема в том, что к моменту появления такого диффа спорить уже поздно: работа сделана, тесты зелёные, отказ означает выбросить сорок минут чужой работы и свои два часа чтения. Психологически проще согласиться. Именно поэтому большие диффы принимают.
Где ошибка дешевле
Одна и та же ошибка стоит по-разному в зависимости от того, на каком этапе её заметили.
Решение «вынести подсчёт времени в отдельный сервис» на этапе плана — это строка, которую я вычёркиваю за десять секунд. То же решение на этапе кода — новый класс, его тесты, изменённые вызовы в четырёх местах, запись в контейнере зависимостей. Вычеркнуть это уже нельзя, можно только переделать.
Отсюда основной приём: сначала план, ревью плана, потом код. Не потому что план ценен сам по себе, а потому что на этапе плана моё «нет» стоит абзац, а не час.
Что должно быть в плане
План в свободной форме бесполезен: получается пересказ задачи другими словами, с которым не поспоришь, потому что он ни к чему не обязывает. План должен быть проверяемым.
Не пиши код. Сначала план.
В плане:
- список шагов, каждый — законченное изменение с рабочим проектом на выходе;
- для каждого шага файлы, которых он касается, и оценка размера в строках;
- решения, которые ты принимаешь за меня: новые классы, новые таблицы,
изменения публичных сигнатур;
- что останется непокрытым тестами.
Остановись после плана.Из четырёх пунктов главный — третий. Он вытаскивает на поверхность то, ради чего вообще нужен план: решения, которые иначе я увижу уже воплощёнными.
На рефакторинге сервиса задач в тот план попала строка «создаю TimeTrackingService, переношу туда четыре метода, интерфейса не ввожу». Я эту строку прочитал и оставил. А вот строку «переношу проверку прав из сервиса в контроллер» вычеркнул сразу, и это стоило мне ровно двух секунд.
Оценка размера в строках — вещь неточная, и я не отношусь к ней как к обещанию. Она нужна для другого: шаг с оценкой в 500 строк почти всегда означает, что внутри спрятаны два шага.
Потолок диффа как параметр задачи
Второй приём проще первого и работает без плана: назвать максимальный размер изменения.
Дифф не больше 200 строк. Если задача не помещается — останови работу
и скажи, на какие части её делить.Число здесь не сакральное. Двести строк — это примерно то, что я читаю внимательно за один подход, не теряя нить. У кого-то это триста, у кого-то сто пятьдесят. Важно, что число есть, потому что без него размер определяется задачей, а задача всегда больше, чем кажется.
Побочный эффект оказался ценнее основного. Ограничение в 200 строк заставляет исполнителя выбирать, что делать в первую очередь, а выбор он объявляет вслух. На пятиэтапном рефакторинге первым шагом были предложены характеризующие тесты — не потому что я просил, а потому что переносить методы в пределах лимита без них не получалось.
Второе, что даёт лимит: переговоры вместо факта. Ответ «задача не помещается, предлагаю разбить на пять» приходит до работы, а не после.
Остановка после каждого шага
Формулировка «сделай по шагам» без явной остановки не работает: шаги выполняются подряд, и на выходе всё тот же большой дифф, только с подзаголовками.
Работает явное «остановись»:
Выполни только шаг 1. Прогони тесты. Остановись и покажи дифф.
Я скажу, продолжать ли.Пять таких проходов дают тот же объём работы, что и один большой, но читаются иначе. В диффе на 200 строк я вижу каждую строку. В диффе на 1240 я вижу структуру и верю, что внутри она такая же, как снаружи.
Замер на одном и том же рефакторинге:
| Заход | Форма | Моё время на ревью | Пропущенных дефектов |
|---|---|---|---|
| Одним куском | 1240 строк | 2 ч | 3 |
| Пять шагов по 200 | 5 диффов | 40 мин | 1 |
Разница во времени объясняется просто. Большой дифф я читаю дважды: первый раз, чтобы понять замысел, второй — чтобы проверить детали. Маленький — один раз, потому что замысел в нём виден сразу.
Коммиты по смыслу
На автономных прогонах, где я не смотрю на каждый шаг, лимит диффа заменяется требованием к коммитам.
Коммить по смысловым шагам, не одним коммитом в конце.
Каждый коммит оставляет проект в рабочем состоянии: тесты зелёные.
Сообщение коммита описывает изменение, а не задачу.Прогон на функции меток дал шесть коммитов на 430 строк: миграция и сущность, привязка к задаче, интерфейс управления, фильтр в списке, отображение на карточке, тесты. Каждый читается отдельно, и это единственная причина, по которой такой PR можно ревьюить.
Требование зелёных тестов на каждом коммите даёт ещё и точки возврата. Пятый шаг оказался неудачным — откат до четвёртого ничего не ломает. Без этого требования откат означает откат всей ветки.
Про сообщения: «реализована функция меток» на шести коммитах подряд — это отсутствие сообщений. Помогает явное «сообщение описывает изменение, а не задачу», иначе название задачи копируется во все шесть.
Где план не спасает
План — не гарантия, а способ удешевить возражение. Три случая, где он не помог.
Первый: план правильный, код от него отклонился. На третьем шаге появился класс, которого в плане не было, — понадобился по ходу, и это выглядело разумно. Разумно и было, но я узнал об этом из диффа, а не из плана. Лечится только ревью диффа, который план не отменяет.
Второй: план формально выполнен, а задача не решена. Пункт «добавить фильтр по метке в список задач» был выполнен фильтром, который не дружил с пагинацией. В плане такого уровня детализации не бывает.
Третий: план длиннее задачи. На правке в тридцать строк ревью плана съедает больше времени, чем ревью самого изменения. Порог у меня примерно на сотне строк ожидаемого диффа: ниже — прошу сразу код.
Ревью диффа не отменяется ни в одном из этих случаев. План снижает вероятность, что в диффе окажется чужая архитектура, но не делает чтение диффа необязательным. Разговоры о том, что при хорошем контракте можно принимать не глядя, я слышу регулярно и каждый раз вспоминаю фильтр, который ломал пагинацию.
Что осталось
Из трёх приёмов — план, лимит, остановка — самым дешёвым оказался лимит. Одна строка в задаче, работает без подготовки, даёт переговоры вместо факта.
План я прошу не всегда, и это осознанно: на знакомых задачах он превращается в ритуал. Признак, по которому решаю, — есть ли в задаче решения, которые я не хочу отдавать. Если новые таблицы, новые публичные интерфейсы или перенос ответственности между слоями — план. Если добавление поля в форму — сразу код с лимитом.
Чего до сих пор не научился делать хорошо — ревьюить план быстро. Читаю его так же придирчиво, как код, и трачу минут пятнадцать там, где хватило бы пяти.
