Чеклист для Code Review

Структурированный чеклист помогает проводить код-ревью последовательно и эффективно. Эта статья предлагает комплексный чеклист для всех аспектов Pull Request.

Вкратце

Чеклист code review: проверяй логику, безопасность, производительность, тесты, документацию и архитектуру. Маленькие PR, четкие описания и конструктивный фидбэк — ключ к успеху.

Краткое описание

Чеклисты для code review представляют собой структурированные списки контрольных точек, которые гарантируют, что все релевантные аспекты Pull Request систематически рассматриваются. Они помогают ревьюерам обеспечить согласованное качество, а авторам сосредоточиться на важных моментах. Хороший чеклист охватывает функциональную корректность, безопасность, производительность, покрытие тестами, документацию и архитектуру.

Чеклист

1. Описание PR и контекст

Четкое описание: что изменилось и почему?
Ссылка на задачу: задача/тикет указаны
Стратегия тестирования: как была протестирована функция?
Скриншоты: при изменениях UI присутствуют
Breaking changes: документированы ли критические изменения?

2. Логика и функциональность

Корректность: делает ли код то, что нужно?
Граничные случаи: обработаны ли edge cases и ошибки
Обработка ошибок: правильно ли обрабатываются исключения
Валидация: входные данные валидируются
Освобождение ресурсов: закрываются ли файлы, соединения

3. Безопасность

SQL Injection: используются ли параметризованные запросы
XSS: экранируются ли пользовательские данные
Аутентификация: только авторизованные доступы
Чувствительные данные: нет ли secrets в коде
Валидация входа: все ли входные данные валидируются

4. Производительность

Запросы БД: избежана ли проблема N+1
Кэширование: используется ли кэширование где нужно
Алгоритм: выбран ли эффективный алгоритм
Memory leaks: нет ли утечек памяти
Асинхронные операции: используется ли неблокирующий I/O

5. Стиль кода и читаемость

Названия: значимые имена переменных и функций
Функции: маленькие, одна ответственность
Комментарии: только ПОЧЕМУ, не ЧТО
Дублирование: соблюдается ли DRY
Linter: нет ошибок linter

6. Тесты

Unit тесты: новая логика протестирована
Integration тесты: протестированы ли API endpoints
Граничные случаи: тесты для edge cases
Coverage: достаточно ли покрытие
Flaky тесты: нет ли нестабильных тестов

7. Документация

README: обновлен ли README при необходимости
API-Docs: документированы ли изменения API
Inline-Docs: документированы ли сложные функции
Changelog: обновлен ли changelog
Migration Guide: документированы ли breaking changes

8. Архитектура и дизайн

SOLID: соблюдаются ли принципы
Когезия: связанное ли вместе в одном модуле
Связанность: слабая ли связь между модулями
Абстракция: правильная ли уровень абстракции
Масштабируемость: масштабируется ли дизайн с требованиями

9. Git практики

Commit messages: используются ли conventional commits
Branch name: описательное ли имя ветки
Commits: логически ли сгруппированы коммиты
Merge strategy: согласована ли стратегия rebase или merge
WIP commits: нет ли WIP-коммитов в финальном PR

10. CI/CD

CI green: прошли ли все проверки
Build: успешен ли build
Tests: пройдены ли все тесты
Lint: нет ошибок linter
Security scan: нет ли уязвимостей

Пример ревью

## Code Review: JWT Authentication

### ✅ Хорошо
- Четкое разделение логики auth и controller
- Подробные unit тесты
- Описание PR содержательно

### ⚠️ Улучшения
- Secret должен читаться из переменных окружения
- Валидацию токена можно вынести в отдельный сервис
- Отсутствуют integration тесты для API endpoints

### ❏ Вопросы
- Почему выбран JWT вместо OAuth2?
- Реализован ли token refresh flow?

### ✏️ Требуемые изменения
- [ ] Читать secret из ENV
- [ ] Добавить integration тесты

Приоритизация

Приоритет	Категория	Фокус
Критический	Безопасность, логика	Блокирует merge
Высокий	Производительность, тесты	Нужно исправить
Средний	Стиль, документация	Можно отложить
Низкий	Замечания	Опционально

Ключевые моменты проверки

Структурированный чеклист для согласованных ревью
Категории: логика, безопасность, производительность, тесты, документация
Приоритизация: критический > высокий > средний > низкий
Интеграция CI/CD как quality gate
Breaking changes должны быть задокументированы

FAQ

1. Что такое чеклист для code review?

Структурированный список контрольных точек для согласованных code review.

2. Какие категории должны быть в чеклисте?

Логика, безопасность, производительность, тесты, документация, архитектура, Git, CI/CD.

3. Как приоритизировать фидбэк?

Критический > высокий > средний > низкий. Критический блокирует merge.

4. Какие критические моменты?

Уязвимости, логические ошибки, отсутствие тестов.

5. Что должно быть в описании PR?

Что, почему, тесты, скриншоты, breaking changes.

6. Как проверять безопасность?

SQL Injection, XSS, аутентификация, secrets, валидация входа.

7. Как проверять производительность?

N+1 запросы, кэширование, алгоритм, утечки памяти, async.

8. Что проверять в тестах?

Unit тесты, integration тесты, edge cases, покрытие, отсутствие flaky тестов.

9. Что проверять в документации?

README, API-docs, inline-docs, changelog, migration guide.

10. Что проверять в Git практиках?

Conventional commits, описательные ветки, логические коммиты.

11. Что проверять в CI/CD?

CI green, build, тесты, lint, security scan.

12. Сколько времени должно занимать ревью?

С чеклистом 15-30 минут для маленьких PR.

13. Нужно ли адаптировать каждый чеклист?

Да, адаптируй под проект и команду.

14. Что делать с breaking changes?

Задокументировать, написать migration guide, обновить changelog.

15. Автоматизированные проверки?

Автоматизируй linter, security scan, тесты в CI/CD.

Дальше в пути обучения Software Quality

Следующая статья в пути Software Quality охватывает Static Code Analysis Tools — автоматизированные инструменты для проверки качества кода, такие как ESLint, SonarQube и Pylint.

Täglicher Artikel