Как стать автором
Обновить
124.12
Skyeng
Крутой edtech с удаленкой для айтишников

Плохие практики код-ревью и немного мемов об этом: обсуждаем, как сделать процесс лучше

Время на прочтение 3 мин
Количество просмотров 13K
Как не убить критикой, но и не скатиться в поверхностное ревью? Жирный PR против кучи мержей — что где выгоднее в зависимости от типа и размера проекта? А вот оно вообще правда нужно? Может, ну его — или автоматизируем/заменим статическим анализом?

Тема благодатная — и мы решили посвятить ей совместный стрим с подкастом SDCast.


1 июля в 17 часов по Москве/Киеву/Минску. В эфир придут хорошие люди (о них под катом) — а еще у нас будут свои голосования ;)

Все началось с этого доклада (вот текстовая версия) от Сергея Жука. Константин KSDaemon Буркалев, автор подкаста, зацепился за тему «а каким должно быть код-ревью». И ребята начали обсуждение в твиттере. Мы решили развернуть из 140 символов полноценную дискуссию — и пригласили людей с разным опытом, чтобы взглянуть на вопрос с разных сторон.

Константин Буркалёв, архитектор, разработчик, автор и ведущий SDCast


Расскажи про самую плохую/вредную практику, которую ты видел, встречал или внедрял.

Самое ужасные, что встречал на своей практике — это две крайности:

  • Первая: «Ой, чё-то тут дофига всего ты накоммитил, 25 файлов, 247+/314- изменений… Оно компилируется? Запускается? Ну, наверное, ок».
  • Вторая крайность — супер-детальное ревью с придиркой к каждой букве, символу, отступу, имени, типу, циклу, паттерну. Крайне демотивирует…

Бывали ли у тебя конфликты из-за код-ревью?

Конечно, как же без них! Они возможны даже в уже устоявшихся командах с четкими процессами.

У меня был случай, когда одна моя подопечная уходила в слезах после моего код-ревью. Зато спустя какое-то время её PR практически не требовали правок, а я мог абсолютно спокойно доверить ей ревью других разработчиков.

Есть ли у тебя любимый мем про код-ревью?

image

Вообще, у меня его не было, но я пошел, изучил, какие мемы есть по теме, — и вот)

Александр SamDark Макаров, разработчик фреймворка Yii, активный участник опенсорс-проектов


Расскажи про самую плохую/вредную практику, которую ты встречал.

Придираться к стилю кода. Одно раздражение…

Твой любимый мем про код-ревью...



Бывали ли у тебя конфликты из-за код-ревью?

Бывали из-за кодстайла: человек правил пробелы, ему говорили про скобки, он правил их, находили ещё пробелы. После 2-3 итераций правок человек устал, всех послал и забил на реквест.

Но мы сделали выводы — и внедрили автоматические фиксы: в Yii 2 это делается при релизе, в Yii 3 прям сразу.

Сергей seregazhuk Жук, бэкенд-разработчик в крупнейшей онлайн-школе Европы и контрибьютор в опенсорс


Твой любимый мем про код-ревью...

Этот тред в твиттере.



Расскажи про самую плохую/вредную практику, которую ты встречал.

Мы часто думаем, что некоторые пул-реквесты нет смысла ревьюить человеку — можно положиться на автоматизированные инструменты. Например, зачем человеку ревьюить фиксы стилей и форматирования? Соблюдение code-style спокойно может проверить какая-то тулза. Но беда в том, что код, который эта тулза будет ревьюить, писал именно человек. И он может ошибаться.

У нас был кейс, когда кто-то в команде тестировал внешний сервис локально, добавил в конфиг доступы от боевого сервиса, протестил и сделал реквест только с форматированием. В реквест попали изменения конфига, в котором были production-доступы. Тесты не падали, линтер не ругался — мержим. Конечно, это был приватный репозиторий, но после этого хотя бы краем глаза мы стараемся просматривать ВСЕ реквесты.

Бывали ли у тебя конфликты из-за код-ревью в команде?

По-настоящему конфликтных ситуаций не помню.

Антон amorev Морев, основатель студии заказной разработки


Бывали ли у тебя конфликты из-за код-ревью?

Да, была ситуация — ждал неделю, когда посмотрят мой код, а когда очередь дошла и в коде нашли косяк, то начали говорить, мол, «из-за тебя релиз откладывается». То есть виноват остался я, ведь «можно было сразу все сделать правильно».


Скриншот из той самой злополучной статьи.

Также была ситуация с клиентом, который прочитал вот эту статью на Хабре и каждый раз, когда была хоть какая-то проблема, ссылался на нее. В один момент он начал погружаться в код, давать свои комментарии,… при этом не имея бэкграунда разработчика, просто прочитав пару статей. С клиентом пришлось расстаться.

Твой любимый мем про код-ревью?


Вот он, как раз из выступления Сергея Жука — мужик проверяет людей на входе, просто проводя по воздуху руками.

Расскажи про самую плохую/вредную практику, которую ты встречал или внедрял.

  • Во-первых, ревью ради ревью в поисках нарушений «паттернов разработки», «красоты кода» и т.д. Сам внедрял — когда пробовал руководить фрилансерами: смотрел их код на предмет красоты, меньше времени оставалось на бизнес-фичи и в итоге код выходил, мягко говоря, не очень качественный.

  • Во-вторых, не понимаю, зачем молиться на code coverage. Код может быть хоть целиком покрыт тестами, но не теми, которыми нужно — и в итоге новая фича может сломать все.

  • В-третьих, ревью только одним человеком, который понимает, как устроен проект, и проводит ревью «по решению руководства». Это комбо из бутылочного горлышка и замыленного глаза.

P.S. Надеемся, вы тоже подключитесь в выходной посреди недели и поделитесь своим опытом, болями и находками: в эфире предусмотрены прямые включения голосом, мы также будем читать комментарии в трансляции — а после лайва соберемся на афте-пати в зуме.

Ищите аудио тут.

P.P.S. Расшифровка появится в этом блоге в августе.
Теги:
Хабы:
+32
Комментарии 64
Комментарии Комментарии 64

Публикации

Информация

Сайт
job.skyeng.ru
Дата регистрации
Дата основания
Численность
1 001–5 000 человек
Местоположение
Россия
Представитель
Alisa Kruglova