Где грань достаточно веской причины для отклонение Pull Request при Code Review?

Question

[Алексей Солодкий]

Вопрос по поводу проведения Code review со стороны тим-лида
Где грань что нужно отправлять на исправление а на что можно просто забивать? Насколько высока должна быть планка качества?

Пример - был написан код (коменты мои):

$averageSumPerMonth = Orders::getInstance()
   ->calculateAveragePricePerMonth(Auth::getAccountId()); // 15 or null
if ($averageSumPerMonth) {
   $averageSumPerMonth = $accountService->getBalanceService()
     ->getAmountString($averageSumPerMonth, 0); // '15 рублей'
}

Строка результата $averageSumPerMonth далее передаётся в шаблон.
Так как кроме этого кода в методе (контроллере) ещё много подобного кода, мне кажется логичным вынести это в отдельный метод чтобы также избавится от переписывания одной переменной значениями разных типов.
Достаточное ли это обоснование чтобы завернуть код или лишние придирки?

Я согласен что никто не умрёт если этот код попадёт в продакшен. Но хочется держать планку качества выше.

Основой вопрос - как понять не перегибаешь ли ты палку.

Answer 1

[Андрей Никифоров]

Мое мнение: если это хотфикс — ок. Но нужно отметить себе, что в будущем этот кусок нужно переделать.
Аналогично — если это поддержка легаси и в будущем это будет просто выкинуто.

В других случаях нужно переписать, чтобы в будущем не напороться при поддержке кода.