Ревью кода без моды: как выбирать решения под команду

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

Что именно решают на ревью

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

Перед комментариями зафиксируйте четыре вещи:

• какая проблема появилась и как её измеряют;
• какие ограничения нельзя нарушить: время ответа, бюджет, совместимость, срок релиза;
• кто будет сопровождать решение после его автора;
• как команда откатит изменение, если оно не сработает.

Если в pull request нет этой информации, первый комментарий лучше посвятить контексту. Обсуждать название класса до понимания цели — значит тратить время на симптомы.

Хорошее решение — не то, которое выглядит современно на диаграмме, а то, которое следующий инженер сможет безопасно изменить.

Сноска. Bus factor — число людей, после ухода которых команда ещё способна поддерживать участок системы без длительной передачи знаний. Чем меньше это число, тем дороже сложная или редкая технология.

Как сравнивать решение с возможностями команды

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

Пять вопросов до одобрения

1. Кто уже умеет это поддерживать? Считайте не только автора изменения, но и людей, которые будут разбирать инциденты, обновлять зависимости и проводить ревью.
2. Как быстро появится второй компетентный человек? Если ответ — «наймём потом», решение увеличивает зависимость от одного специалиста.
3. Что произойдёт при сбое? Нужны понятные логи, метрики, резервный путь и процедура отката, а не только уверенность разработчика.
4. Какова цена отказа от текущего подхода? В неё входят миграция данных, переписывание тестов, обучение, изменение CI и поддержка двух вариантов во время перехода.
5. Можно ли проверить гипотезу малым изменением? Если да, сначала делайте ограниченный эксперимент, а не меняйте фундамент приложения.

Фраза «это стандарт индустрии» не отвечает ни на один из вопросов. Стандарт может быть оправдан для найма и интеграций, но не отменяет стоимость внедрения.

Четыре стратегии рефакторинга

Ревью часто ломается на выборе масштаба. Один участник предлагает заменить библиотеку целиком, другой — ничего не трогать, пока не случится авария. Между этими позициями есть несколько рабочих вариантов.

Полную замену стоит одобрять только вместе с планом миграции, критериями успеха и датой удаления старого пути. Если старый и новый код останутся навсегда, команда не завершила рефакторинг — она добавила второй продукт к первому.

Как выбирать между локальным и большим изменением

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

Не выбирайте новый инструмент только потому, что в нём проще написать демонстрационный пример. На ревью важнее полный жизненный цикл: тестирование, обновления, диагностика, релиз и откат.

Как писать полезные замечания

Комментарий должен объяснять риск и предлагать способ проверить гипотезу. Вежливость важна, но сама по себе не делает замечание полезным.

Формулировки, которые двигают решение

• Вместо: «Так не принято». Пишите: «Этот вызов выполняется внутри обработки каждого запроса. При росте нагрузки он добавит повторное обращение к хранилищу. Можно ли вынести результат за пределы цикла и добавить метрику количества обращений?»
• Вместо: «Нужен другой паттерн». Пишите: «Сейчас бизнес-правило зависит от конкретного клиента внешнего сервиса. Если оставить интерфейс на границе модуля, тесты не будут запускать сеть. Есть ли причина не отделить этот клиент?»
• Вместо: «Перепишите всё». Пишите: «Предлагаю сначала покрыть текущий сценарий тестом, затем заменить внутреннюю реализацию. Так мы проверим, что рефакторинг не изменил ответ для существующих входных данных».
• Вместо: «Эта библиотека моднее». Пишите: «У нового решения пока нет владельца в команде. Кто будет обновлять его и разбирать сбои? До внедрения стоит провести небольшой эксперимент и описать план поддержки».

Разделяйте обязательные замечания и предложения. Ошибка, которая приводит к потере данных, не должна выглядеть так же, как предпочтение другого имени переменной. Удобная маркировка — блокирующее, важное и необязательное замечание, если такие уровни приняты в вашей команде.

Что ломается в рефакторинге

Самые дорогие проблемы обычно возникают не в строках, которые меняли, а на границах системы.

• Скрытый контракт. Другой сервис зависит от формата ответа, порядка событий или побочного эффекта, который нигде не описан.
• Два источника истины. Во время миграции старый и новый код по-разному сохраняют одну сущность.
• Неполный откат. Приложение можно вернуть к старой версии, но схема базы или формат очереди уже изменились.
• Потеря наблюдаемости. Новый путь работает, но в нём нет прежних метрик, корреляционных идентификаторов и понятных ошибок.
• Рост когнитивной нагрузки. После «улучшения» инженеру приходится знать два фреймворка, два способа конфигурации и два набора исключений.

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

Где подход ломается

Стратегия «делать маленькими шагами» не спасает, если каждый шаг меняет публичный контракт или данные уже повреждены. Адаптер не решает проблему, когда обе реализации имеют разные гарантии доставки и команда не умеет сравнивать их результаты. Локальный рефакторинг тоже может стать ловушкой: если одинаковая проблема повторяется в десятках модулей, исправление одного места только создаёт иллюзию прогресса.

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

Что делать дальше

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

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