Код-ревью: как проходить и как проводить, чтобы не ссориться
Код-ревью это то место, где вежливые люди неожиданно начинают спорить о скобках. Причина обычно не в скобках, а в том, что никто не договорился, зачем вообще ревью делается и что в нём проверяется.
Разберём обе стороны: как читать чужой код и как переживать правки к своему.
Зачем оно на самом деле
Поиск багов тут только третья по важности задача. Автоматические тесты и линтеры находят больше и дешевле.
Первое: распространение знаний. После ревью в коде разбирается не один человек, а минимум двое. Это единственная страховка от ситуации, когда автор увольняется и модуль превращается в чёрный ящик.
Второе: удержание единого стиля решений. Не форматирования (для этого есть автоформаттеры), а подходов: как обрабатываем ошибки, где держим бизнес-логику, как называем сущности.
Третье: собственно баги и краевые случаи, которые тесты не покрыли.
Если в команде ревью воспринимают как экзамен, оно превращается в источник напряжения. Если как способ вместе довести изменение до ума, работает.
Что смотреть в чужом пулл-реквесте
Порядок примерно такой, от важного к мелочам.
Решает ли задачу. Открыть тикет, прочитать, сверить. Удивительно часто выясняется, что код прекрасен, но делает не то.
Краевые случаи. Пустой список, ноль, отрицательное значение, дубли, отсутствующие права, недоступный внешний сервис. Самое ценное, что даёт ревьюер.
Ошибки и отказы. Что произойдёт, если запрос упадёт на середине. Есть ли ретраи, не приведут ли они к дублям, залогировано ли достаточно, чтобы потом разобраться.
Данные и миграции. Совместима ли миграция со старым кодом во время выкатки, не блокирует ли она таблицу надолго, есть ли откат.
Тесты. Не количество, а смысл: проверяют ли они поведение или просто повторяют реализацию.
Читаемость. Понятно ли это будет через полгода человеку, который не участвовал в обсуждении.
Мелочи стиля. Последним и лучше автоматикой. Спорить о кавычках в комментариях к ревью это трата времени всех участников.
Как писать комментарии
Тон решает больше, чем содержание.
Отделяйте обязательное от желательного. Пометки вроде «блокер», «предложение», «мелочь» экономят всем нервы: автор сразу видит, что чинить обязательно, а что на его усмотрение.
Спрашивайте, а не приговаривайте. «Тут не обработается пустой список?» лучше, чем «ты забыл проверить на пустоту». Иногда выясняется, что не забыл, а список не может быть пустым по контракту выше.
Объясняйте причину. «Давай вынесем в отдельную функцию» без объяснения читается как вкусовщина. С объяснением («её потом переиспользуют в отчёте») это уже аргумент.
Хвалите вслух. Заметили удачное решение, напишите об этом. В ленте из двадцати замечаний одна похвала меняет тональность всего обсуждения.
Не переписывайте чужой код в комментариях целиком. Если правок больше десяти, идите разговаривать голосом, это быстрее и дешевле.
Как принимать ревью своего кода
Правило номер один: замечания к коду это не оценка вас. Звучит банально, но именно тут спотыкается большинство конфликтов.
Отвечайте на все комментарии, даже коротким «поправил». Молчаливое исправление оставляет ревьюера в неведении.
Спорьте, когда есть аргумент. Ревьюер не всегда прав, и «сделаю как сказали, хотя считаю иначе» плохой исход для обоих. Приведите довод, обсудите.
Разбивайте большие изменения. Пулл-реквест на две тысячи строк не ревьюят, его пролистывают и ставят апрув. Если хотите настоящей проверки, режьте на куски по несколько сотен строк.
Пишите описание. Что меняется, зачем, что проверяли руками, на что обратить внимание. Пять строк описания экономят ревьюеру двадцать минут.
Сколько ждать и что делать с зависшим ревью
Норма это несколько часов, максимум день. Дальше изменение устаревает, конфликты нарастают, автор переключается на другое и забывает контекст.
Если ревью зависает регулярно, проблема процессная, а не личная: нет договорённости, кто и когда смотрит. Лечится явным правилом вроде «смотрим до обеда» или ротацией дежурного ревьюера.
Что спрашивают про ревью на собеседовании
Вопрос звучит буднично: «как у вас устроено код-ревью?» За ним обычно проверяют три вещи.
Понимаете ли вы, зачем оно нужно, или просто соблюдаете ритуал.
Как реагируете на критику. Ответ в духе «у нас все замечания по делу, я всегда соглашаюсь» звучит подозрительно ровно. Живой ответ включает пример спора и того, чем он закончился.
Умеете ли смотреть чужой код. Часто следом просят рассказать, на что вы обращаете внимание в чужом пулл-реквесте, и тут пригодится список выше.
Иногда дают практическое задание: фрагмент кода с ошибкой, найдите и объясните. Формат встречается и в банках вопросов, и на живых секциях, потому что он показывает больше, чем теория.
Если ревью в команде нет
Бывает и такое, особенно в маленьких командах и на проектах, где всё держится на одном человеке.
Начинать проще снизу: попросите коллегу глянуть ваш код, не оформляя это как процесс. Один-два раза, потом привыкают. Формальный регламент без привычки обычно умирает через месяц.
А заодно это хорошая тренировка перед собеседованием: чтение чужого кода развивает именно тот навык, который проверяют на секции с разбором фрагмента.
Кстати, в Сеньорчике часть вопросов сделана именно в таком формате: даётся кусок кода, надо найти ошибку или выбрать корректный вариант правки. Плюс обычные вопросы по языку, базам, тестам и архитектуре, с разбором каждого. Десять минут в день в Telegram, начать можно бесплатно.