• Зачем и как проводить Code Review

    И ваши QA это пропустили???
    QA lead после этого не был уволен???
    При чём тут вообще код ревью?

    Учимся мыслить логически. Мой коммент был на то, где человек говорит что ни один CR не ускорит разработку. Перефразирую мои слова — если бы таска прошла код ревью, а не была докинута где-то в конце релиза тайком, то этого бы *скорее всего* не было. И не нужно было тратить огромное кол-во времени на откат, фикс багов и переделку.
    А при чем тут вообще QA? У вас есть 20 фич в спринте, вы их сделали, QA проверили эти 20 фич. Все чотко. Тут перед самым релизом человек в тот же PR что был проревьюван докидает какой-то рефактор. Бабах, релиз пошел по жопе. Или у вас QA еще перед релизом регрессию всего затронутого функционала делают? Очч интересно)

    Кто со мной не согласен — тот джун?

    Опять же, невнимательно/бездумно читаете. Я в примере привел ситуацию с джунами. Окей, замените на любое другое слово. 2 миддла пусть будет. Сильно поменяло смысл? При чем это вообще?) Имелось ввиду, что если хоть кто-то в команде становится не согласен с ревью то считать его неправильным — откровенно глупо.

    Причём тут код ревью?

    Хмм, блин, забыл, на логику ж не нужно смотреть. На реализацию тоже. Задача ревьювера — быть линтером, проверять скобочки и точки с запятыми. Хз, у нас на проекте ревьювер старается вникнуть в логику того что написано и делает ли этот код вообще то что задумывалось.

  • Зачем и как проводить Code Review

    PR: Зачем и как проводить Code Review
    Status: Rejected with comments:

    Более того, нет такого CR, который смог бы ускорить вашу разработку. В любом случае вам придется потратить время на создание Pull Request (PR), исправление багов после ревью, проверку на работоспособность функционала после исправления и так далее.

    да ну? видел огромное кол-во примеров, когда таску нужно было откатывать из релиза и переделывать как раз из-за того что человек не отправил ее на ревью, а подкинул в релиз и релиз гай то пропустил. После этого тратиться много времени и сил на исправление проблем (например, созданные 10к неправильных заказов, или 2кк дубликатов продуктов), а также фикс логики в самой задаче.
    второй момент — PR можно создавать не только когда закончишь задачу, но и *сюрприз* в процессе, итерируясь небольшими подзадачами, получая фидбек и советы по реализации в процессе. Не знаю где вы там что ревьюваете, но что-то пошло не так.

    Когда поймете что есть что-то за пределами кода и на самом деле задача состоит в решении бизнес проблемы (которую нужно решить в том числе качественно, без багов и огромных архитектурных дыр), а не просто в быстром написании какого-то кода, который вроде работает и покрывает 2 тест кейса в Jira, то откроете для себя интересный мир.

    Правильный CR — CR, проведение которого было согласовано командой, были учтены пожелания или рекомендации сотрудников и оговорены определенные правила его проведения, о которых я расскажу дальше.

    Эмм. Допустим я человек отвечающий за ревью. У нас есть общие принципы проведения ревью, проверенные годами и кровью/потом поколений разработчиков до нас. На этом этапе оно еще правильное.
    Ко мне в команду приходят 2 джуна, которые не согласны с какими-то принципами ревью (например, «зачем, я и так все знаю, вы мне не доверяете? тратить время, мог бы поиграть в теннис, бла-бла»). Внезапно, CR в нашей компании становится не правильным, ибо 2 джуна с этим не согласны?)

    Может вызывать конфликты между разработчиками в моменты, когда не получается прийти к консенсусу.

    за 7 лет разработки, из них 5 лет с код ревью, последние 3 года помогаю с ревью и ревьюваю сам. Еще не видел ситуаций когда не приходили к консенсусу и были холивары по реализации.
    Хотяяя, 1 раз когда 2 архитектора друг на друга по приколу батхертили в комментах, но думаю это не нужно считать. Может проблема не в CR, а в команде?

    Может занимать довольно много времени на обсуждение реализации и ее исправление, вследствие чего будет страдать скорость разработки продукта.

    Нет, не может. Почему? Потому что первичное и основное обсуждение реализации должно быть на этапе планирования. Если ваша финальная реализация настолько плоха, возможно проблема также не в CR, а в подходе к разработке и стадии планирования?

    И опять же, сделанный код != разработка (развитие) продукта. Опять же, нужно смотреть на уровень выше. А решает ли этот код проблемы бизнеса? Или после релиза вылезет 200 сайдеффектов и багов, которые принесут бизнесу много потерь в деньгах и репутации.

    чем больше изменений за раз, тем сложнее откатывать код при такой необходимости.

    VCS в помощь, делается 1 кнопкой во всех популярных IDE, даже не нужно в терминал заходить.

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

    вспоминаем про планирование и обсуждение задачи с более опытными коллегами до реализации

    Не взваливайте на себя слишком много ревью.
    Не задерживайте ревью.

    пункты так-то взаимоисключающие друг друга

    Не ведитесь на обещания. Если видите проблемные места и разработчик кормит вас обещаниями «Я это обязательно исправлю в следующем PR» или «Я создам себе отдельную задачу на правку», как показывает практика, 99%, что за этот функционал забудут, так как проект не стоит на месте и новые задачи никуда не пропадут. Настаивайте на всех важных правках здесь и сейчас.

    опять же, раньше было отличать хотелки от необходимостей. Если вы готовы пропустить кусок кода в продакшин, возможно, это не необходимость. Простейшее решение — топаем к PM, говорим что есть проблема и нужно Х часов на доп разработку, выносим в отдельную задачу? Все, проблема решена, дальше идем пить смузи и кататься на моноколесах по офису

    Что-то слишком много расплодилось пустых статей про CR. Формулу как проводить код ревью мы не сформировали.
    Вместо всей этой полотнины с проблемами, которые вообще никак не связаны с CR, можно было описать все в 2х предложениях:
    Не вдавайтесь в крайности, будьте вежливы, советуйтесь с опытными коллегами во время реализации. Выработайте практику код ревью которая будет работать для вашей команды и следуйте ей.
    Остальное люди и так должны понимать, взрослые же, не?

    Поддержали: Oleg Klimenko, Sergiy Morenets
  • Code review: принципы проведения, основные правила и рекомендации

    о чем эта статья?
    просто рассказ о каком-то абстрактном ревью кода? все (ну или почти все) кому нужно ревьювать код и так это знают
    лучше бы описали на что обращаете внимание при ревью, как происходят ревью функционала затрагивающего разные куски системы (ну не бывает такого чтобы не приходилось рефакторить или внедрять более-менее глобальные интеграции). Вся эта фигня про маленькие атомарные таски это больше про проекты уровня мобильных приложений. Если у вас огромный E-commerce или B2B решение, то так или иначе будете выходить за эти рамки атомарных задачек.
    Как поступаете в случае если нужно идти на компромиссы (классический пример — есть задача А, для нее нет хорошего решения и рецензент написал плохое, но работающее решение, в простонародье костыль. Можно потратить условно x человеко-часов на поиск хорошего решения (что возможно нужно отдельно согласовать с заказчиком и он может не аппрувнуть доп расходы на разработку), или пустить в прод текущее решение. Как вы поступите? Будете до последнего не пускать в продакшин код? или встанете на сторону зла?)
    Интересуют примеры хорошего тона при ревью или разные граничные кейсы.
    это про заглавие статьи, если что, вот про это вот

    В этой статье я поделюсь нашим опытом code review в контексте GameDev

    Где тут опыт, как это вообще относиться к GameDev?

    По самой статье я очень сомневаюсь что есть люди, которые:
    — вместо описания проблемы в комменте к ревью пишут просто «все х-ня, переделать»
    — переходят на личности в комментах к PR
    — просто пишут что это плохо без описания причины или примера failure, пускай сам угадывает
    — (рецензенты) отправляют на ревью не глядя, даже не пробегая по коду глазами
    Это как выдать статью что не нужно пить воду из лужи. Вроде и правильно, но адекватные люди сами понимают что так делать не нужно.

    В нашей команде мы договорились о времени суток для ревью — утро каждого рабочего дня

    это круто, конечно, если в PR все отлично
    а если там список предложений по рефактору то на следующее утро откладываете? (не думаю что рецензент пофиксит и проверит все в тоже утро) а если несколько этапов нужно для проверки? затягивается таска на неделю? ее как-бы нужно еще протестировать перед релизом, может даже демо показать, а спринт уже заканчивается
    если задача критическая? например, баг в приложении который нужно срочно исправить и откатывать последний релиз не вариант. Тоже на утро отложите? Вот у меня не получается разграничивать и очень часто приходиться перепрыгивать на ревью для вот таких граничных ситуаций. Было бы интересно послушать ваши подходы к такому (если у вас вообще бывают такие проблемы).

    Кроме того, в процессе code review можно выявить критические ошибки, которые в конечном итоге могут привести к серьезным багам. Можно сделать вывод о том, что ревью кода существенно повышает ценность цикла разработки для создания качественных продуктов.

    это воообще как? вы каждый PR пулите, компилируете и запускаете на локальной машине, а затем тестируете/дебажите всю логику? Вот сколько не ревьюваем в команде, самые опасные баги — незаметные, которые чаще всего без автотестов или регрессий не заметишь. И каким образом такие баги увидеть на Github? Можно на каждый PR тратить огромное кол-во времени и всю логику и кейсы воспроизводить в голове, либо действительно запускать. Но это вообще не юз-кейс для ревью)

    Сотрудничество и обмен новыми методами. В основном инженеры работают обособленно. Проверка кода способствует сотрудничеству и взаимодействию внутри команды относительно кода и обмену мыслями. Члены команды могут обмениваться информацией о новых знаниях во время проверки кода. Это помогает участникам проекта повышать свои навыки, узнавая о новейших технологиях, а также выстраивать доверительные отношения, что обязательно сказывается на конечном результате.

    это очень плохо когда в команде обмен мыслями происходит не на этапе планирования и разработки функционала, а уже на этапе код ревью. Особенно узнать про новые технологии когда уже задача закончена и нужно переделывать под эту технологию (я так понимаю это имелось ввиду, или как?)
    Про доверительные отношения тоже непонятно. Если я, как ревьювер, вижу ошибки и перепроверяю код членов моей команды, то каким образом я буду развивать доверительные отношения с ними? Также, если буду постоянно реджектить и заставлять переделывать куски функционала, то как это развивает доверительные отношения? Может это из следующей статьи про тим билдинг и корпоратив пункт вырван?