Шаг 0: Подготовка (для автора). Прежде чем создавать pull request (PR), убедитесь, что ваш код готов к показу. Это включает: выполнение всех существующих тестов, написание новых тестов для своего функционала, проверку код-стайла проекта (используйте линтеры), обновление документации (комментарии, README, API docs). В описании PR четко укажите: что было изменено, почему (ссылка на issue или проблема), как было протестировано. Для нашего примера возьмем гипотетический PR в проект утилиты для работы с файлами, добавляющий новую опцию фильтрации по расширению.
Шаг 1: Первичный обзор (для ревьювера). Не бросайтесь сразу в код. Прочтите описание PR и связанный issue. Поймите контекст и цель изменений. Затем выполните код локально. Проверьте, что изменения компилируются/запускаются, а тесты проходят. Это базовая проверка "работоспособности".
Шаг 2: Проверка архитектуры и дизайна. Это самый важный этап. Оцените, гармонично ли новое решение вписывается в существующую архитектуру проекта. В нашем примере: добавлена ли новая опция фильтрации в правильное место? Не дублирует ли она существующую функциональность? Правильно ли расширены интерфейсы (сигнатуры функций)? Не нарушены ли принципы SOLID? Например, если фильтрация была реализована через стратегию, новая опция должна быть добавлена как еще одна конкретная стратегия, а не через гигантский условный оператор в основном методе.
Шаг 3: Анализ самого кода. Теперь погрузитесь в детали. Проверяйте:
- Читаемость: понятны ли имена переменных и функций? (`filter_by_extension` лучше, чем `fbe`).
- Простота: нельзя ли код упростить? Нет ли избыточных вычислений?
- Обработку ошибок: проверяются ли некорректные входные данные (например, пустое расширение)? Кидаются ли понятные исключения?
- Безопасность: нет ли уязвимостей (например, возможность path traversal через подставное расширение?).
- Производительность: нет ли неоптимальных алгоритмов, например, линейный поиск в цикле, который можно заменить на хэш-таблицу?
Шаг 4: Проверка тестов. Тесты — это спецификация поведения. Достаточно ли их? Покрывают ли они не только "счастливый путь", но и граничные случаи (расширение с точкой, без точки, в верхнем/нижнем регистре)? Не являются ли тесты хрупкими (зависят от внешнего состояния)? В PR должен быть добавлен тест для новой функциональности.
Шаг 5: Формулировка обратной связи. Комментарии должны быть конструктивными, конкретными и доброжелательными. Вместо "Этот код ужасен" напишите: "Метод `processFile` стал очень большим. Можно ли вынести логику фильтрации в отдельный класс, как это сделано для `FilterBySize`? Это улучшит читаемость и соответствие Open-Closed principle". Используйте вопросы: "Как ты думаешь, что произойдет, если пользователь передаст `*.txt`?" Похвалите хорошие решения — это мотивирует.
Шаг 6: Дискуссия и итерация. Автор отвечает на комментарии, вносит правки. Ревьювер проверяет новые коммиты. Дискуссия ведется прямо в интерфейсе PR. Важно сохранять уважительный тон и фокусироваться на коде, а не на личности автора. Если возникают разногласия по архитектуре, можно привлечь третьего разработчика или тимлида.
Шаг 7: Финализация. Когда все серьезные замечания устранены и код соответствует стандартам проекта, ревьювер ставит аппрув (approve). Важный принцип: "Два глаза видят лучше, чем один". В многих mature open-source проектах требуется аппрув от нескольких мейнтейнеров. После этого PR может быть вмержен в основную ветку.
Такой системный подход превращает code review из рутины в мощный инструмент обучения, распространения знаний по кодовой базе и непрерывного улучшения качества продукта, что ярко демонстрирует практика успешных open-source сообществ.
Комментарии (9)