Ответ
На прошлой работе код-ревью проводилось через Pull Requests (PR) в GitHub. Процесс выглядел так:
- Разработчик создает PR, назначает ревьюверов (обычно 1-2 коллеги из команды)
- Ревьюверы проверяют код на:
- Соответствие код-стайлу (через анализаторы вроде SonarQube)
- Наличие Unit-тестов для новой логики
- Оптимальность алгоритмов
- Возможные edge-cases
- Комментарии оставляются прямо в PR, обсуждаются в треде
- После исправлений PR апрувится и мержится
Пример комментария в ревью:
// Вместо этого:
foreach(var item in list) { ... }
// Лучше использовать LINQ:
list.ForEach(item => ... );Важные нюансы:
- Время ответа на PR не должно превышать 1 рабочего дня
- Критика только по коду, без перехода на личности
- Обязательно отмечать и хорошие решения тоже
Ответ 18+ 🔞
А, ну расскажу, как у нас на прошлой работе этот цирк с код-ревью устраивали. Через эти ваши Pull Requests в GitHub, блядь. Процедура, мать её, священная, как в аптеке.
Вот пишешь ты код, потеешь, думаешь, что гениальность чистой воды. А потом — хуяк — создаёшь PR и назначаешь пару несчастных коллег ревьюверами. Обычно одного-двух, чтобы не очень распылялись. И начинается, ёпта, самое интересное.
Сидят эти товарищи и выискивают в твоём творении, как собаки сучками, всё подряд:
- Код-стайл. Тут без вариантов — если анализатор типа SonarQube орёт, что ты криво отступ поставил, то всё, пиши пропало. «Фикси, блядь, отступы, глаза кровоточат!»
- Юнит-тесты. Добавил новую логику? А где, сука, на неё тесты? Без них — ни шагу. «Хочешь в продакшен? Докажи, что эта штука не сломает всё к херам при первом же чихе пользователя».
- Алгоритмы. Тут могут придраться по-серьёзному. «А че это у тебя сложность O(n²), а? Ты что, совсем, блядь, с дуба рухнул? Переписывай на O(n log n), и чтобы я этого не видел!»
- Edge-cases. Это святое. «А если сюда
nullприлетит? А если список пустой? А если сервер упадёт, а пользователь — инопланетянин?» В общем, проверяют на вшивость со всех сторон.
Все претензии пишут прямо в PR, в комментах. И там же, в треде, идёт обсуждение — иногда конструктивное, а иногда просто трёп до посинения. Но, справедливости ради, ругались редко, больше по делу.
Вот, например, классический комментарий, который каждый второй получал:
// Вместо этого:
foreach(var item in list) { ... }
// Лучше использовать LINQ, чувак, не изобретай велосипед:
list.ForEach(item => ... );И главное — были негласные правила, которые все знали, но которые иногда хуёво соблюдались:
- Время ответа — не больше рабочего дня. А то бывало, зальёшь PR, а он висит сутки, и все делают вид, что не при делах. Бесило, блядь, конкретно.
- Критика только по коду. Личности не трогаем. Не «ты — мудак», а «это решение — мудацкое». Чувствуешь разницу? Хотя иногда, конечно, срывало крышу, но старались держаться.
- Хвалить тоже надо. Если решение офигенное — обязательно сказать. А то одни упрёки, так вообще руки опустятся. Поддержка, мать её, моральная.
В общем, процесс рабочий, но иногда такой бюрократичный, что просто ебать. Зато, справедливости ради, в продакшен меньше говна летело.