Информация об изменениях

Сообщение Re: Не люблю ревьюить чужой код от 29.05.2023 15:27

Изменено 29.05.2023 15:29 Pauel

Re: Не люблю ревьюить чужой код
Здравствуйте, Философ, Вы писали:

Ф>Во-первых, мне тяжело что-либо ревьюить код, если я не вник в суть задачи.


А зачем это нужно? Если сильно хочется вникать да контролировать плотно, то выгоднее идти через парно-программирование.

Если такой роскоши нет, то на ревью можно завести чек-лист, и каждый ревьюер будет идти по нем.
— размер пр. Если слишком много — пусть автор заводит фича бранч, сплитает фичу на промежуточные пр.
— есть ли валидация параметров публичных методов
— есть ли тесты — юниты, функциональные, e2e
— корректна ли документация на методы
— изобретены ли велосипеды
— именование публичных методов
— есть ли обработка ошибок
— корректно ли оформлены конфиги
— есть ли магические константы итд
— мутный код вида "а херлиты в базу лезешь сразу из контролера"
— секурити типа "распаковывать oidc токен без валидации ой-ой-ой" создан тикет №10123

До кучи, правила ревьюеров, запретить всё ниже
— переделай, измени, убери — вместо этого интервьюер сообщает, какая проблема с кодом, если надо, то создаёт тикет. Если же хочется идти по варианту "переделай, измени, убери", то парно-программирование будет лучше.
— это не секюрно. Вместо этого интервьюер сообщает, почему не секюрно, и создает тикет.
— идиотский код. Вместо этого интервьюер задает вопросы, уточняет и тд.
— нет форматирования итд. Для этого есть автоматические тулы

То есть, вместо два-три дня на ревью быстрый проход по чеклисту, на выходе список тикетов. Мелкие и критические тикеты исправляются сразу, остальные уходят в баклог как технические задачи. Дальше ПР мергается в транк.
Re: Не люблю ревьюить чужой код
Здравствуйте, Философ, Вы писали:

Ф>Во-первых, мне тяжело что-либо ревьюить код, если я не вник в суть задачи.


А зачем это нужно? Если сильно хочется вникать да контролировать плотно, то выгоднее идти через парно-программирование.

Если такой роскоши нет, то на ревью можно завести чек-лист, и каждый ревьюер будет идти по нем.
— размер пр. Если слишком много — пусть автор заводит фича бранч, сплитает фичу на промежуточные пр.
— есть ли валидация параметров публичных методов
— есть ли тесты — юниты, функциональные, e2e
— корректна ли документация на методы
— изобретены ли велосипеды
— именование публичных методов
— есть ли обработка ошибок
— корректно ли оформлены конфиги
— есть ли магические константы итд
— мутный код вида "а херлиты в базу лезешь сразу из контролера"
— секурити типа "распаковывать oidc токен без валидации ой-ой-ой" создан тикет №10123

До кучи, правила ревьюеров, запретить всё ниже
— переделай, измени, убери — вместо этого интервьюер сообщает, какая проблема с кодом, если надо, то создаёт тикет. Если же хочется идти по варианту "переделай, измени, убери", то парно-программирование будет лучше.
— это не секюрно. Вместо этого интервьюер сообщает, почему не секюрно, и создает тикет.
— идиотский код. Вместо этого интервьюер задает вопросы, уточняет и тд.
— нет форматирования итд. Для этого есть автоматические тулы

То есть, вместо два-три дня на ревью быстрый проход по чеклисту, на выходе список тикетов. Мелкие и критические тикеты исправляются сразу, остальные уходят в баклог как технические задачи. Дальше ПР мергается в транк.

Идея в том, что бы кодревью прошел быстро, и бранча не тухла неделями. Иначе проблем она создаст гораздо больше. Чем позже мерж, тем больше геморроя при мерже-ребейзе и тем больше скрытых багов