про КодРевью
От: snaphold  
Дата: 14.04.18 05:13
Оценка:
Исходя из темы по соседству, понял что код ревью совсем не помешает.
Так получилось что сейчас у нас его нет и комментарии по коду бывают только, когда смотришь выборочные коммиты или наталкиваешься на непонятный код.

Проекту 5 человеколет. Писало >5 программистов. код стайла не было.
Люди привыкли писать безконтрольно и добавлять разные длл и новые паттерны когда им надо.
Гдето работа с базой через энтити, где то процедуры адо.нет.
В итоге имеем зоопарк.

Как внедрить код ревью, максимально безболезненно?
И как вообще должен выглядеть этот процесс?
В команде 8 человек, включая дба.
Re: про КодРевью
От: neFormal Россия  
Дата: 14.04.18 08:43
Оценка: +3
Здравствуйте, snaphold, Вы писали:

S>Как внедрить код ревью, максимально безболезненно?

S>И как вообще должен выглядеть этот процесс?
S>В команде 8 человек, включая дба.

довольно простой вариант — это ввести пулреквесты и запрет на мерж в мастер до ревью.
дополнительно к этому прикладываются требования соблюдать стиль кода и ограничения на некоторые паттерны.
...coding for chaos...
Re: про КодРевью
От: bnk СССР http://unmanagedvisio.com/
Дата: 14.04.18 10:25
Оценка:
Здравствуйте, snaphold, Вы писали:

S>Исходя из темы по соседству, понял что код ревью совсем не помешает.

S>Так получилось что сейчас у нас его нет и комментарии по коду бывают только, когда смотришь выборочные коммиты или наталкиваешься на непонятный код.

S>Проекту 5 человеколет. Писало >5 программистов. код стайла не было.

S>Люди привыкли писать безконтрольно и добавлять разные длл и новые паттерны когда им надо.
S>Гдето работа с базой через энтити, где то процедуры адо.нет.
S>В итоге имеем зоопарк.

S>Как внедрить код ревью, максимально безболезненно?

S>И как вообще должен выглядеть этот процесс?
S>В команде 8 человек, включая дба.

Когда кто-то начинает работать над задачей, он создает новую ветку для себя из основной (мастера)
Основная идея в том, что свою ветку нельзя просто взять и замержить в мастер, тем более нельзя коммитить прямо в мастер, все изменения в мастере только через пул реквест (за исключением военного времени, понятно).
И этот пул реквест должен посмотреть и заапрувить другой разработчик, который этот код (изменения) способен понять (или тимлид, для новичков — чтобы быть построже). После того, как реквест заапрувлен, он мержится в мастер.

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

Вообще для этого лучше всего какой-то готовый инструментарий,который поддерживае ревью пулл реквестов.
Например, в мире microsoft это TFS, последние версии весьма неплохо работают.
У atlassian это свой стек (jira +crucible). У github свой.

Настройка этого для TFS например называется branch policy.
Просто вешаешь политики на мастер, и дело в шляпе
https://docs.microsoft.com/en-us/vsts/git/branch-policies-overview?view=vsts
https://docs.microsoft.com/en-us/vsts/git/branch-policies?view=vsts
Отредактировано 14.04.2018 10:58 bnk . Предыдущая версия . Еще …
Отредактировано 14.04.2018 10:41 bnk . Предыдущая версия .
Отредактировано 14.04.2018 10:37 bnk . Предыдущая версия .
Отредактировано 14.04.2018 10:33 bnk . Предыдущая версия .
Отредактировано 14.04.2018 10:27 bnk . Предыдущая версия .
Re: про КодРевью
От: notacat  
Дата: 14.04.18 11:35
Оценка:
S>Исходя из темы по соседству, понял что код ревью совсем не помешает.
S>Так получилось что сейчас у нас его нет и комментарии по коду бывают только, когда смотришь выборочные коммиты или наталкиваешься на непонятный код.

S>Проекту 5 человеколет. Писало >5 программистов. код стайла не было.

S>Люди привыкли писать безконтрольно и добавлять разные длл и новые паттерны когда им надо.
S>Гдето работа с базой через энтити, где то процедуры адо.нет.
S>В итоге имеем зоопарк.

S>Как внедрить код ревью, максимально безболезненно?

S>И как вообще должен выглядеть этот процесс?
S>В команде 8 человек, включая дба.
чтобы люди серьезно к этому относились, а не просто галочку ставили, что было код ревью, надо чтобы оно им лично было нужно. В этом смысле полезно подумать про коллективное владение кодом. Меняться иногда задачами. Если кто-то будет менять что-то в чужом коде — вот тут код ревью и понадобится, чтобы не сломать что-то ненароком. И если команда начнет на этом фоне больше общаться, то рано или поздно они про код стайлы тоже задумаются. Или легче воспримут, если кто-то предложит что-то такое использовать.
Re[2]: про КодРевью
От: snaphold  
Дата: 14.04.18 12:47
Оценка:
Здравствуйте, neFormal, Вы писали:

F>Здравствуйте, snaphold, Вы писали:


S>>Как внедрить код ревью, максимально безболезненно?

S>>И как вообще должен выглядеть этот процесс?
S>>В команде 8 человек, включая дба.

F>довольно простой вариант — это ввести пулреквесты и запрет на мерж в мастер до ревью.


а про пулреквесты можно подробнее?
Мерж в мастер надо контролировать вручную или гитом настраивается?

F>дополнительно к этому прикладываются требования соблюдать стиль кода и ограничения на некоторые паттерны.


вот боюсь погрязнуть в постоянном ежедневном анализе кода. а если делать раз в неделю может оказаться что много переделывать нужно будет
Re[3]: про КодРевью
От: bnk СССР http://unmanagedvisio.com/
Дата: 14.04.18 13:12
Оценка:
Здравствуйте, snaphold, Вы писали:

S>а про пулреквесты можно подробнее?

S>Мерж в мастер надо контролировать вручную или гитом настраивается?

Ууу как все запущено
Конечно же это должна система делать. Работал с github? Примерно так же. Если не работал, то попробуй, чтобы понять, как оно должно быть. В общем, кроме гит (или другой vcs), нужен софт котрый над ним и поддерживает ревью.

Можно конечно пул реквесты по почте друг другу посылать, но в 2018 году есть варианты получше.

Что у вас есть из инфраструктуры? Git, как я понял. Что-то еще есть? Система сборки (CI)? Багтрекер?

S>вот боюсь погрязнуть в постоянном ежедневном анализе кода. а если делать раз в неделю может оказаться что много переделывать нужно будет


Зачем тебе-то этим заниматься? Ревью должен делать тот, кто может его сделать.
Самый простой вариант — другой разработчик, который тоже работает с модулем, для которого изменение.
Административно-технически достаточно настроить ограничение, что любой добавляемый код должен быть просмотрен как минимум кем-то еще перед тем как он вливается в общую базу.

Код, который комитится джунами (если они у вас есть), рекомендую таки просматривать самому (как тимлиду), потому как многие делают этот ревью сквозь пальцы (хотя все равно при этом удаляется достаточное количество косяков).
Но вообще ревью полезен, даже сквозь пальцы. И технически (объяснять другому, нафига очередную говнобиблиотеку в код притащил), и функционально (установка бита х была архиважна для системы у, а чувак по незнанию системы у ее вынес), и случайных (забыл раскомментировать), ну и с образовательной точки зрения (учишься у других что как делать)
Отредактировано 14.04.2018 13:50 bnk . Предыдущая версия . Еще …
Отредактировано 14.04.2018 13:47 bnk . Предыдущая версия .
Отредактировано 14.04.2018 13:44 bnk . Предыдущая версия .
Отредактировано 14.04.2018 13:41 bnk . Предыдущая версия .
Отредактировано 14.04.2018 13:24 bnk . Предыдущая версия .
Отредактировано 14.04.2018 13:21 bnk . Предыдущая версия .
Отредактировано 14.04.2018 13:14 bnk . Предыдущая версия .
Re[4]: про КодРевью
От: snaphold  
Дата: 14.04.18 13:54
Оценка:
Здравствуйте, bnk, Вы писали:

bnk>Здравствуйте, snaphold, Вы писали:


S>>а про пулреквесты можно подробнее?

S>>Мерж в мастер надо контролировать вручную или гитом настраивается?

bnk>Ууу как все запущено

bnk>Конечно же это должна система делать. Работал с github? Примерно так же. Если не работал, то попробуй, чтобы понять, как оно должно быть.

bnk>Что у вас есть из инфраструктуры? Git, как я понял. Что-то еще есть? Система сборки (CI)? Багтрекер?



Запущено, да))
Есть только Git)
Хотим багтрекер прикрутить всё.
А вообще если расскажите по постановке процесса был бы признателен. Если несложно в деталях.

S>>вот боюсь погрязнуть в постоянном ежедневном анализе кода. а если делать раз в неделю может оказаться что много переделывать нужно будет


bnk>Зачем тебе-то этим заниматься? Ревью должен делать тот, кто может его сделать.


bnk>Самый простой вариант — другой разработчик, который тоже работает с модулем, для которого изменение.


а если другой забьет на качество и подстроиться под это? типа проект и так Г. и пытаться начинать конфетку смысла нет
Re[5]: про КодРевью
От: neFormal Россия  
Дата: 14.04.18 15:03
Оценка:
Здравствуйте, snaphold, Вы писали:

S>Есть только Git)


даже с "только гитом" можно сделать ревью.
это решается административным способом. т.е. просто договариваетесь про личный отсмотр.
мы так делали: http://rsdn.org/forum/management/5999675.1
Автор: neFormal
Дата: 31.03.15


S>>>вот боюсь погрязнуть в постоянном ежедневном анализе кода.


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

S>а если другой забьет на качество и подстроиться под это? типа проект и так Г. и пытаться начинать конфетку смысла нет


что есть "г."? ради чего ты хочешь сделать ревью?
если много багов, которые отнимают время; если нужно быстрей обучать новичков; если нужно привести проект к единому стилю, чтобы быстрее писать, то ревью поможет.
если же вы просто ходите на работу, вас не увольняют за баги, вас не поджимают сроки, вы не теряете в деньгах... то ревью может и не нужно. "хорошо сидим".
сводить личные счёты через ревью — плохая идея. если цель не благородная, то команда не поймёт и не примет.
...coding for chaos...
Re[5]: про КодРевью
От: neFormal Россия  
Дата: 14.04.18 15:12
Оценка:
Здравствуйте, snaphold, Вы писали:

S>А вообще если расскажите по постановке процесса был бы признателен. Если несложно в деталях.


ах, да. процесс:
1. заводится задача в таск-трекере
2. под задачу заводится ветка
3. в ветке проводятся работы, делается коммит и пул-реквест.
4. реквест отсматривают и мержат. в разных системах можно делать ограничения: например, минимум два аппрува для мержа
5. задача помечается, как resolved здесь или в п.3.
6. после тестирования QA или другой ответственный человек её закрывает. если выделенного тестера нет, можно закрыть сразу и ждать багрепортов.

инструментарий для старта — это, наверное, gitlab. для тех, у кого есть деньги и желание — jira.
для гитлаба лучше поставить плагином redmine в качестве таск-трекера(встроенный очень ограниченный).
думаю, все трекеры следят за ветками через имена. т.е. в имени ветки указывается номер задачи(также привязаться может по номеру в коммите). это делается встроенными скриптами, которые вешаются на хуки в гите.
...coding for chaos...
Re[5]: про КодРевью
От: bnk СССР http://unmanagedvisio.com/
Дата: 14.04.18 20:58
Оценка: +1
Здравствуйте, snaphold, Вы писали:

bnk>Что у вас есть из инфраструктуры? Git, как я понял. Что-то еще есть? Система сборки (CI)? Багтрекер?


S>Запущено, да))

S>Есть только Git)
S>Хотим багтрекер прикрутить всё.
S>А вообще если расскажите по постановке процесса был бы признателен. Если несложно в деталях.

Если вас там 10 человек, просто возьмите готовую систему онлайн — Microsoft VSTS или Atlassian Jira/Bitbucket например за 10 баксов.
Зачем разводить дома зоопарк, гораздо удобнее когда вся инфраструктура для разработки уже идет в комплекте, включая багтрекер, ревью, сборку.
Репозиторий Git пушится в облако на раз-два.

S>а если другой забьет на качество и подстроиться под это? типа проект и так Г. и пытаться начинать конфетку смысла нет


Он вполне может оказаться прав. Если в результате ревью усложнится жизнь разработчиков, то он не нужен.
Обычно разработчики пишут плохой код не специально, а просто по незнанию, или ввиду обстоятельств непреодолимой силы.
А благодаря ревью можно разделить ответственность за код с кем-то еще.
Отредактировано 14.04.2018 23:00 bnk . Предыдущая версия . Еще …
Отредактировано 14.04.2018 22:57 bnk . Предыдущая версия .
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.