Расскажите о своем опыте проведения code review.

Ответ

Да, я регулярно участвую в code review как в роли автора, так и в роли ревьюера. Для меня это важный процесс для поддержания качества кода, обмена знаниями и коллективного владения кодовой базой.

При проверке кода я обращаю внимание на следующие аспекты:

  1. Корректность и логика: Соответствует ли код поставленной задаче, нет ли в нем логических ошибок.
  2. Архитектура и дизайн: Не нарушает ли код общую архитектуру проекта, можно ли его упростить.
  3. Читаемость и стиль: Код должен быть понятным для других разработчиков. Я слежу за соблюдением go fmt и командных соглашений, осмысленностью имен переменных и функций.
  4. Производительность: Анализирую наличие узких мест, неэффективных алгоритмов, лишних аллокаций памяти.
  5. Обработка ошибок: Убеждаюсь, что ошибки не игнорируются (_), а корректно обрабатываются и, при необходимости, оборачиваются для добавления контекста.
  6. Конкурентность: Проверяю на наличие потенциальных race conditions, утечек горутин, правильность использования примитивов синхронизации (мьютексы, каналы).
  7. Безопасность: Ищу потенциальные уязвимости, такие как SQL-инъекции, некорректная обработка пользовательского ввода и т.д.
  8. Тестируемость: Оцениваю, насколько легко написать юнит-тесты для нового кода.

Пример частого замечания (идиоматичный Go):

// Было: классический цикл в стиле C
func processItems(items []string) {
    for i := 0; i < len(items); i++ {
        // do something with items[i]
    }
}

// Стало: использование for-range, что более идиоматично для Go
// Это не только короче, но и безопаснее (нет риска ошибки в индексе).
func processItems(items []string) {
    for _, item := range items {
        // do something with item
    }
}

Я всегда стараюсь давать конструктивную обратную связь, объясняя причину замечания и предлагая варианты решения.

Ответ 18+ 🔞

А, code review, говоришь? Ну это ж святое дело, блядь! Как без него? Это ж как в армии без дедовщины — можно, конечно, но тогда кто тебе, сука, объяснит, что ты мудак и пишешь хуйню?

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

Ну, смотри, на что я там обычно пялюсь, когда чужой код разглядываю:

  1. Работает ли оно вообще, или это просто арт-объект? Первым делом смотрю — решает ли эта писанина ту задачу, которую должна. А то бывает, чувак такой красивый алгоритм накрутит, а оно нихуя не пашет, как задумано.
  2. Архитектура. Не лепит ли он костыли к нашей красивой, блядь, системе? Не превращает ли модуль в божественный объект, который всё знает и всё умеет? Иногда проще переписать кусок, чем воткнуть ещё одну заплатку.
  3. Читаемость, ёпта! Это главное. Код должен читаться как газета, а не как шифровка из штаба. Имена переменных a, b, c — это пидарашный подход, извини. go fmt — наш бог, его заветы чти. Если после твоего кода у коллег глаза начинают кровить — ты что-то сделал не так.
  4. Не тормозит ли всё в хлам? Смотрю, нет ли там, блядь, квадратичных алгоритмов на больших данных, или он память аллоцирует, как сумасшедший, в цикле. Иногда одно sync.Pool может спасти ситуацию.
  5. Ошибки, сука! Вот это моя любимая тема. Увидел if err != nil { _ = err } — сразу красная карточка, нахуй! Игнорирование ошибок — это корень всех зол, ёбаный в рот! Ошибки нужно либо обрабатывать, либо явно пробрасывать наверх с контекстом, чтобы потом не гадать, откуда ноги растут у этого nil pointer dereference.
  6. Горутины и мьютексы. Ах, эта сладкая конкурентность! Проверяю, нет ли расовых гонок, утечек горутин (которые потом как черти из коробочки выскакивают) и чтобы мьютексом не лочили целое приложение на полчаса.
  7. Безопасность. Ну тут понятно. Чтобы никаких сырых SQL-строк с подстановкой пользовательского ввода, а то потом «ой, у нас базу слили».
  8. А тесты к этому говну написать можно? Если функция принимает десять параметров и меняет три глобальные переменные — это пиздец, а не код. Его не протестировать.

Вот, смотри, классический пример, который просто бесит:

// Было: чувак, видимо, с Java или C пришёл, пишет как умеет.
func processItems(items []string) {
    for i := 0; i < len(items); i++ {
        // делаем что-то с items[i]
    }
}

// Стало: да ты что, ёпта! В Go же есть for range! Это ж красиво и безопасно!
// Никаких тебе ошибок на границах массива.
func processItems(items []string) {
    for _, item := range items {
        // делаем что-то с item
    }
}

Главное в ревью — не быть ебучим занудой-педантом, который придирается к пробелам. Нужно объяснить, почему так лучше, и, если что, предложить свой вариант. Мы же все в одной команде, в конце концов, а не на конкурсе «Кто умнее». Хотя иногда, блядь, так и тянет написать «я ебал это читать, переделывай». Но надо держать себя в руках, как тот самый Герасим, пока Муму не принесли.