Добавить в корзинуПозвонить
Найти в Дзене

Разбор худшего пул-реквеста в моей карьере

За годы код-ревью я видел всякое, но один пул-реквест от мидла заставил меня проверить, не взломали ли наш гит. Задача была простой. Починить падение внешней интеграции в корзине. буквально 20 строк кода. Вместо этого автор принёс полторы тысячи строк изменений в десяти модулях. Он решил, что раз уж залез в код, то заодно перепишет всю логику оформления заказа, обновит старые библиотеки и внедрит модный паттерн, о котором только утром прочитал в интернете. Внутри этого хаоса творился настоящий ад. Вместо аккуратного перехвата ошибок там красовалась конструкция из пяти вложенных try-catch, которая просто глушила любые исключения, а в логи выводила лаконичное «Ой». Вдобавок для оптимизации автор воткнул сырой SQL-запрос с дырой для инъекций прямо внутрь цикла, из-за чего тестовый стенд мгновенно ложился под минимальной нагрузкой. Но вишенкой на торте стали упавшие тесты. Вместо того чтобы разобраться, почему его глобальный рефакторинг всё сломал, разработчик просто закомментировал все м

За годы код-ревью я видел всякое, но один пул-реквест от мидла заставил меня проверить, не взломали ли наш гит. Задача была простой. Починить падение внешней интеграции в корзине. буквально 20 строк кода. Вместо этого автор принёс полторы тысячи строк изменений в десяти модулях. Он решил, что раз уж залез в код, то заодно перепишет всю логику оформления заказа, обновит старые библиотеки и внедрит модный паттерн, о котором только утром прочитал в интернете.

Внутри этого хаоса творился настоящий ад. Вместо аккуратного перехвата ошибок там красовалась конструкция из пяти вложенных try-catch, которая просто глушила любые исключения, а в логи выводила лаконичное «Ой». Вдобавок для оптимизации автор воткнул сырой SQL-запрос с дырой для инъекций прямо внутрь цикла, из-за чего тестовый стенд мгновенно ложился под минимальной нагрузкой.

Но вишенкой на торте стали упавшие тесты. Вместо того чтобы разобраться, почему его глобальный рефакторинг всё сломал, разработчик просто закомментировал все мешающие проверке тесты, чтобы пролезть сквозь пайплайн сборки. Мол они всё равно устарели, после релиза перепишу.

Мы закрыли этот кошмар без слияния, а ветку удалили. Саму задачу в итоге переписали за полчаса, но этот случай подарил нам елезобетонное правило команды. Один пул-реквест - одна атомарная задача. Никакого партизанского рефакторинга. Это лучшая прививка от благих намерений, которые едва не похоронили релиз.