Почему Code Review в smartnut.ru?

    Привет, Хабр! Я — один из команды разработчиков SmartNut, web-сервиса для небольших компаний, занимающихся поддержкой и it-аутсорсингом. Мы гордимся тем, что мы делаем и ничуть не меньше тем, как мы это делаем. Хочу поделиться с вами историей из реальной жизни на тему code review.

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

    Как у нас процесс выглядел раньше

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

    Как стало

    Многие из нас знакомы с практиками eXtreme Programming. Одна из них — это парное программирование. По опыту очень сложно полноценно использовать данную практику — сложно отказаться от “личного пространства”, сложно “продать” эту практику начальству (“программисты будут работать в два раза медленнее?!”) и т.п.
    Но можно начать есть слона по частям. Первая часть — code review!

    Программист Вася выполнил задачу. Чтобы задача считалась выполненной, нужно, чтобы она прошла через колонку code review на нашей scrum доске.

    Таким образом мы:
    • получаем анализ кода другим программистом ещё до коммита;
    • улучшаем дизайн кода;
    • передаём знания на ранней стадии;
    • избавляемся от некоторого количества очевидных ошибок, невидимых для замыленного глаза Васи.

    А чтобы не было вопросов “кто должен провести Code Review?” каждую итерацию у нас есть дежурный по проведению данной процедуры.

    Мы не используем какой-либо дополнительный инструментарий для Code Review, а всегда садимся вместе и просматриваем изменения.
    Использование каких-то инструментов в данном случае только замедлит процесс. Если вы находитесь в одной комнате, то незачем заниматься email-пинг-понгом — намного проще и понятнее объяснить всё словами, а лучше — показать. К тому же, личное общение намного полезнее для развития каждого сотрудника и для команды в целом.

    Так мы делаем smartnut.ru и вам того же желаем.

    Будьте гибче, друзья, и до новых встреч!
    ITSM 365
    Company
    Ads
    AdBlock has stolen the banner, but banners are not teeth — they will be back

    More

    Comments 17

      0
      Еще удобно использовать github для code review, можно оставлять комментарий на любую строчку кода
        0
        возможно, если команда распределённая, но если вы находитесь в одном помещении, то наиболее эффективным вариантом будет непосредственный просмотр кода у автора.
        про минусы CR после коммита я написал.
          0
          используем пул реквесты на гитхабе для этих целей и :octocat:
          Проводить ревью кода устно не всегда удобно. Бывает, что переделки нужны немаленькие. Каждый раз отслеживать и запоминать список того, что нужно изменить и что изменили тоже сложно. Выход: писать заметки по коду, следить за коммитами. На гитхабе это делать очень удобно.
          И смотреть код должен обязательно более квалифицированный сотрудник, чем тот, кто писал этот самый код, иначе смысла от такого код ревью будет немного.
            0
            > Каждый раз отслеживать и запоминать список того, что нужно изменить и что изменили тоже сложно
            Не очень понятно — мы просто смотрим исходящие изменения в Eclipse, ничего запоминать не нужно, все изменения видны.
            Основная же суть review до коммита (как и тестирования до коммита) — разработчик не успевает отключиться от контекста задачи; ему не нужно тратить время, чтобы вспомнить чем он занимался.

            > И смотреть код должен обязательно более квалифицированный сотрудник, чем тот, кто писал этот самый код, иначе смысла от такого код ревью будет немного
            Т.е. у самого квалифицированного сотрудника код смотреть никто не будет? :) Как написал коллега в статье, мы рассматриваем code review ещё и как элемент парного программирования. И поверьте, более квалифицированному частенько есть чему поучиться у всех остальных.
              0
              смотреть каждый коммит тоже накладно. Мы проверяем фитчу целиком, когда она закончена и вносим поправки. Ведь бывает, что код не сразу формируется и некоторые участки могут быть изменены самим разработчиком несколько раз, прежде чем он решит, что код готов. Да и дергать каждый раз сотрудников, на то чтобы они посмотрели код, тоже не дело. Я смотрю код тогда, когда у меня есть время, а до этого момента пул реквест висит и никому не мешает (опять же это обычная ветка с которой можно в любой момент мержиться).
              Насчет того что разработчику не нужно переключать контекст при вашем подходе, ошибаетесь, ему то не нужно, а тому кто будет смотреть код — нужно.
              Если вашему более квалифицированному сотруднику приходится учиться у ваших низкоквалифицированных сотрудников, то у меня для вас плохие новости :).
                0
                т.е. я
                  0
                  пардон, опечатался.
                  Т.е. я правильно понимаю, что у вас на фитчу идет один коммит? И перед коммитом идет обязательный ревью? А если фитча содержит 10 коммитов?
                    0
                    Да, у нас на одну задачу идёт один коммит. А перед коммитом — обязательно проводится code review и тестирование. Только после этих двух операций код можно складывать в общий репозиторий.

                    > А если фитча содержит 10 коммитов?
                    Мы стараемся «распиливать» задачи таким образом, чтобы ни одна из них не занимала более трёх дней. Конечно, бывают исключения, но в целом мы справляемся.
                    По поводу количества коммитов — обычно это действительно один коммит.
                      0
                      позвольте я угадаю, вы используете svn, который отложил такой отпечаток на ваш процесс?
                  0
                  > Если вашему более квалифицированному сотруднику приходится учиться у ваших низкоквалифицированных сотрудников, то у меня для вас плохие новости :)

                  Например?

                  «приходится учиться» — какое-то странное определение. У каждого человека существует миллион привычек, выработанных годами, а работа в паре позволяет смотреть не только на код, но и даже на то, как человек работает с мышкой, например или какие хоткеи использует. Ну говоря уже о более сложных вещах.

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

                  Считать же что «более квалифицированный сотрудник» по определению знает абсолютно всё, что знает «мене квалифицированный» — по-моему, бред.
                    0
                    Нет ничего плохо в узнавании нового. Плохо это когда код ревью используют для этих целей. Можно на досуге посмотреть чужой код, найти для себя что-то новое, это собственно никак не должно быть привязано к код ревью. Он в себе несет совершенно другие цели.
                    0
                    > Насчет того что разработчику не нужно переключать контекст при вашем подходе, ошибаетесь, ему то не нужно, а тому кто будет смотреть код — нужно.

                    Именно поэтому у нас есть «дежурный по code review» — за одну итерацию переключением контекста страдает только один человек :)
                      0
                      дежурный по код ревью, такой бред мог породить действительно только процесс основаный на ущербности репозитория.
                        0
                        уважаемый uzver, позвольте поинтересоваться, каким образом Вы «встраиваете качество» в результат Вашей деятельности? какими практиками пользуетесь в процессе и для улучшения процесса разработки?
                          0
                          Практика, которой мы следуем описана здесь scottchacon.com/2011/08/31/github-flow.html.
                          Основной принцип: получить максимум продуктивности от всех участников проекта, а не привести всех в одному знаменателю. Под продуктивностью подразумевается кол-во «качественного» кода в единицу времени.
                            0
                            правильно ли я понимаю, что у Вас, отсутствует «команда» как таковая? есть разрозненные (географически или по другому принципу) участники проекта, которые никак не взаимодействуют, не обмениваются опытом кроме как через код?

                            как Вы меряете «качество», какие средства и подходы используете для подсчёта «качественного кода в единицу времени»?
                              0
                              Вся команда находится территориально в одном городе и офисе. Код пишется и из офиса и из дома и из другого города и даже когда нет соединения с сетью вовсе, днем/ночью, в любое время. В этом плане мы ничем не ограничены.
                              Качество кода мы измеряем кол-вом матных слов сказаных в единицу времени при ревью этого кода.
                              Обмен опытом у нас происходит в любое удобное время и когда есть желание, время и мотивация (обеих сторон).

              Only users with full accounts can post comments. Log in, please.