#quality
О том, как сделать ревью не таким обидным для разработчиков написана не одна статья, поэтому сегодня коснемся другой стороны ревью - как выжать из этого процесса максимум пользы для команды. Благодаря коллегам, стало понятно, что профит бизнесу приоритетнее чем техническая составляющая.
Как выглядело мое ревью раньше: смотрел построчно каждые изменения в ПР, проверял, что тесты проходят и код выглядит ок. Классы на местах, логика последовательна. После - апрувил и был доволен. Вместо этого, попробуйте придерживаться иной последовательности, когда в следующий раз будете ревьювить ПР коллеги.
Понимание задачи
Начните с главного, понимания задачи, которую пытается решить разработчик. Важно: поймите бизнес требование, а не текущую реализацию. Проверьте, разбиваются изменения на 2 и больше пулл реквеста или нет. Если сложно - попросите коллегу о помощи или подробном описании того, зачем изменения нужны. На этом этапе определяется насколько сложную и значимую задачу решал разработчик, а ревьюверы погружается в контекст проблемы. Правило, которого стараюсь придерживаться: 1 пулл реквест == 1 атомарная задача, которую просто понять и просто объяснить на каждом из уровней абстракции.
Проверка, что задача выполнена
Следующий шаг - проверить, что бизнес задача, которой занимается разработчик, завершена. Важно не путать бизнес задачу и работу ради работы. Проверять стоит в интеграционных тестах. Я стороник пирамиды тестирования фаулера. Поэтому считаю, что интеграционные тесты не должны проверять пограничные случаи.
Проверка архитектуры и способа решения
Когда появилась уверенность, что бизнес задача решена, стоит опуститься на абстракцию ниже и посмотреть, как реализован код. Места куда стоит смотреть: изоляция функций между собой, где лежит логика, мешается логика между собой или нет. Также стоит проверить наличие юнит тестов для каждого класса и метода. Здорово, если юнит тесты покрывают граничные случаи и неправильное использование кода (не тот тип данных, как пример)
Проверка кода и грамматики
Последний шаг - скрупулезная проверка кода. Тут проверяется нейминг, стилистические особенности, проверяется перформанс.
Подитожив, можно увидеть такую последовательность:
Понимание задачи -> проверка, что задача выполнена -> проверка архитектуры и способа решения -> проверка кода и грамматики
О том, как сделать ревью не таким обидным для разработчиков написана не одна статья, поэтому сегодня коснемся другой стороны ревью - как выжать из этого процесса максимум пользы для команды. Благодаря коллегам, стало понятно, что профит бизнесу приоритетнее чем техническая составляющая.
Как выглядело мое ревью раньше: смотрел построчно каждые изменения в ПР, проверял, что тесты проходят и код выглядит ок. Классы на местах, логика последовательна. После - апрувил и был доволен. Вместо этого, попробуйте придерживаться иной последовательности, когда в следующий раз будете ревьювить ПР коллеги.
Понимание задачи
Начните с главного, понимания задачи, которую пытается решить разработчик. Важно: поймите бизнес требование, а не текущую реализацию. Проверьте, разбиваются изменения на 2 и больше пулл реквеста или нет. Если сложно - попросите коллегу о помощи или подробном описании того, зачем изменения нужны. На этом этапе определяется насколько сложную и значимую задачу решал разработчик, а ревьюверы погружается в контекст проблемы. Правило, которого стараюсь придерживаться: 1 пулл реквест == 1 атомарная задача, которую просто понять и просто объяснить на каждом из уровней абстракции.
Проверка, что задача выполнена
Следующий шаг - проверить, что бизнес задача, которой занимается разработчик, завершена. Важно не путать бизнес задачу и работу ради работы. Проверять стоит в интеграционных тестах. Я стороник пирамиды тестирования фаулера. Поэтому считаю, что интеграционные тесты не должны проверять пограничные случаи.
Проверка архитектуры и способа решения
Когда появилась уверенность, что бизнес задача решена, стоит опуститься на абстракцию ниже и посмотреть, как реализован код. Места куда стоит смотреть: изоляция функций между собой, где лежит логика, мешается логика между собой или нет. Также стоит проверить наличие юнит тестов для каждого класса и метода. Здорово, если юнит тесты покрывают граничные случаи и неправильное использование кода (не тот тип данных, как пример)
Проверка кода и грамматики
Последний шаг - скрупулезная проверка кода. Тут проверяется нейминг, стилистические особенности, проверяется перформанс.
Подитожив, можно увидеть такую последовательность:
Понимание задачи -> проверка, что задача выполнена -> проверка архитектуры и способа решения -> проверка кода и грамматики