Ответ
Да, я регулярно участвую в code review как в роли автора, так и в роли ревьюера. Для меня это важный процесс для поддержания качества кода, обмена знаниями и коллективного владения кодовой базой.
При проверке кода я обращаю внимание на следующие аспекты:
- Корректность и логика: Соответствует ли код поставленной задаче, нет ли в нем логических ошибок.
- Архитектура и дизайн: Не нарушает ли код общую архитектуру проекта, можно ли его упростить.
- Читаемость и стиль: Код должен быть понятным для других разработчиков. Я слежу за соблюдением
go fmtи командных соглашений, осмысленностью имен переменных и функций. - Производительность: Анализирую наличие узких мест, неэффективных алгоритмов, лишних аллокаций памяти.
- Обработка ошибок: Убеждаюсь, что ошибки не игнорируются (
_), а корректно обрабатываются и, при необходимости, оборачиваются для добавления контекста. - Конкурентность: Проверяю на наличие потенциальных race conditions, утечек горутин, правильность использования примитивов синхронизации (мьютексы, каналы).
- Безопасность: Ищу потенциальные уязвимости, такие как SQL-инъекции, некорректная обработка пользовательского ввода и т.д.
- Тестируемость: Оцениваю, насколько легко написать юнит-тесты для нового кода.
Пример частого замечания (идиоматичный 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, говоришь? Ну это ж святое дело, блядь! Как без него? Это ж как в армии без дедовщины — можно, конечно, но тогда кто тебе, сука, объяснит, что ты мудак и пишешь хуйню?
Я в этой роли и автора, и проверяющего регулярно болтаюсь. И знаешь, это ж не просто придирки ради придирок, а реально полезная хуйня. Чтобы код не превращался в свалку, где только автор, да и то через неделю, понимает, как эта штука работает.
Ну, смотри, на что я там обычно пялюсь, когда чужой код разглядываю:
- Работает ли оно вообще, или это просто арт-объект? Первым делом смотрю — решает ли эта писанина ту задачу, которую должна. А то бывает, чувак такой красивый алгоритм накрутит, а оно нихуя не пашет, как задумано.
- Архитектура. Не лепит ли он костыли к нашей красивой, блядь, системе? Не превращает ли модуль в божественный объект, который всё знает и всё умеет? Иногда проще переписать кусок, чем воткнуть ещё одну заплатку.
- Читаемость, ёпта! Это главное. Код должен читаться как газета, а не как шифровка из штаба. Имена переменных
a,b,c— это пидарашный подход, извини.go fmt— наш бог, его заветы чти. Если после твоего кода у коллег глаза начинают кровить — ты что-то сделал не так. - Не тормозит ли всё в хлам? Смотрю, нет ли там, блядь, квадратичных алгоритмов на больших данных, или он память аллоцирует, как сумасшедший, в цикле. Иногда одно
sync.Poolможет спасти ситуацию. - Ошибки, сука! Вот это моя любимая тема. Увидел
if err != nil { _ = err }— сразу красная карточка, нахуй! Игнорирование ошибок — это корень всех зол, ёбаный в рот! Ошибки нужно либо обрабатывать, либо явно пробрасывать наверх с контекстом, чтобы потом не гадать, откуда ноги растут у этогоnil pointer dereference. - Горутины и мьютексы. Ах, эта сладкая конкурентность! Проверяю, нет ли расовых гонок, утечек горутин (которые потом как черти из коробочки выскакивают) и чтобы мьютексом не лочили целое приложение на полчаса.
- Безопасность. Ну тут понятно. Чтобы никаких сырых SQL-строк с подстановкой пользовательского ввода, а то потом «ой, у нас базу слили».
- А тесты к этому говну написать можно? Если функция принимает десять параметров и меняет три глобальные переменные — это пиздец, а не код. Его не протестировать.
Вот, смотри, классический пример, который просто бесит:
// Было: чувак, видимо, с 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
}
}
Главное в ревью — не быть ебучим занудой-педантом, который придирается к пробелам. Нужно объяснить, почему так лучше, и, если что, предложить свой вариант. Мы же все в одной команде, в конце концов, а не на конкурсе «Кто умнее». Хотя иногда, блядь, так и тянет написать «я ебал это читать, переделывай». Но надо держать себя в руках, как тот самый Герасим, пока Муму не принесли.