Что проверяют при ревью кода
Что проверяют
Есть ли у вас собственные критерии ревью, или вы ставите аппрув, пробежав код глазами. Интервьюер хочет понять, отличаете ли вы то, что ловит машина, от того, что видит только человек. Следом часто спрашивают, как вы пишете замечания и что делаете, если автор не согласен.
Как отвечать
Начните с разделения: форматирование, порядок импортов и типы проверяют Prettier, ESLint и tsc --noEmit в CI. Спорить в ревью о точке с запятой значит тратить время людей на работу линтера. Человек смотрит на остальное:
- правильность: делает ли код то, что в задаче, обработаны ли пустой список, ошибка сети, долгий ответ, двойной клик;
- состояние и эффекты: нет ли
useEffect, который синхронизирует то, что можно вычислить при рендере, полны ли зависимости, есть ли очистка подписок и таймеров, стабильны лиkeyв списках; - безопасность:
dangerouslySetInnerHTMLбез санитайзера вроде DOMPurify, токены вlocalStorage, секреты в переменных, которые попадут в бандл; - доступность: кнопка это
button, а неdivсonClick, у полей есть подписи, всё работает с клавиатуры; - производительность: новая зависимость на сотни килобайт ради одной функции, тяжёлый экран без ленивой загрузки, лишние перерисовки длинных списков;
- тесты и читаемость: покрыта ли новая логика, понятны ли имена, не дублирует ли код готовый хук или утилиту.
Про форму замечаний: помечайте уровень, nit: для необязательного и явное «блокирует» для того, без чего мёржить нельзя. Замечание адресовано коду, а не автору, с причиной и вариантом исправления. PR на тысячи строк честно просить разбить: качество ревью падает с размером.
Где спотыкаются
Перечисляют только стиль и нейминг. Это работа линтера, а интервьюер ждёт логику, крайние случаи и архитектуру.
Не смотрят изменения вживую. Для UI нужно открыть превью или запустить ветку: по диффу не видно, что форма теряет введённые данные при ошибке.
Путают ревью с переписыванием под свой вкус. Если решение корректно и понятно, «я бы сделал иначе» не повод блокировать PR.