Должны ли ваши лучшие программисты проверять чужой код в системе контроля версий?

29

Одно из различий между svn и git - это возможность контролировать доступ к хранилищу. Сложно сравнивать эти два, потому что есть разница во взглядах на то, кому вообще разрешено вносить изменения!

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

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

GlenPeterson
источник
5
Определить «лучшего программиста»? Лучший в чем? Следовать произвольным правилам? Проверять код? Написание кода с нулевым дефектом?
Тимо Гевш
3
Извините, я все еще пытаюсь осмыслить концепцию проверки неконтролируемого (то есть не проверенного) кода ... несомненно, одно из ключевых преимуществ использования SCS заключается в том, что проверка может быть проведена в отношении известного / контролируемого итерация кода?
Андрей
2
@ У Эндрю с gitлюбым разработчиком может быть свое собственное репо (на его персональном компьютере) и общедоступное личное репо (на сервере позади apache), к которому он может только добавлять изменения. Разница заключается в том, что только репо ведущих разработчиков является «блаженным», из которого каждый должен оформить заказ. Лид-код извлекает из общедоступных репозиториев разработчика и объединяет их с публичным репозиторием Вы оба знали и управляли итерацией, а также контролем исходного кода.
Юбер Карио
32
«Похоже, Git полагает, что только ваши лучшие (самые продуктивные, самые опытные) программисты могут проверять код» - неверное предположение. Git можно настроить так, как вы хотите. Модель «Запрос на извлечение» - это всего лишь один из способов, который идеально подходит для проектов с открытым исходным кодом и потенциально большим числом неизвестных участников. В большинстве коммерческих сред модель «запроса извлечения» будет иметь красный флаг, указывающий на плохие процессы и процедуры SDLC и QC.
Mattnz
4
Я считаю, что @mattnz здесь правильный. Это исключительно результат сильного влияния открытого кода на git, где есть основная команда разработчиков, которая контролирует состояние репо, но другие также могут внести свой вклад.
Стивен Эверс

Ответы:

53

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

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

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

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

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

Карл Билефельдт
источник
10
+1: Для ... Хорошие программисты тратят много времени на работу с неработающим кодом других программистов.
Джим Г.
3
+1 Лучший ответ. Особенно подчеркивая, что один разработчик, совершающий ошибку, нарушающую сборку, негативно влияет на всех.
Эван Плейс
Оказавшись в этой ситуации, оказалось, что два лучших программиста были заняты полный рабочий день для проверки и исправления чужого кода. Конечно, качество кода на VCS было хорошим, но моральное состояние этих двоих уменьшилось быстрее, чем унитаз. То, что начиналось как, казалось бы, хорошая идея, превратилось в кошмар, когда эти двое выбежали за дверь в места, где они могли бы получить, скажем, более творческие задания.
Newtopian
1
Это хороший момент, @Newtopian. В тех местах, где я видел этот успех, больше микросервисной модели, и только одна команда scrum имеет доступ к любому микросервису, но ответственность распространяется на систему в целом. Если у вас нет хотя бы пары опытных программистов на команду, ваша практика найма нуждается в улучшении.
Карл Билефельдт
40

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

С одной стороны, он сделал свое дело. Ряд плохих коммитов был обнаружен до того, как они попали в кодовую базу (и быстро забывались на неделю или около того, пока кто-то не наткнулся на них). Это вызвало меньше нарушений в кодовой базе. Кроме того, я мог бы отодвинуть некоторые вещи форматирования / структуры, прежде чем они стали технологическим долгом; поймать несколько ошибок, прежде чем они стали ошибками. И это дало мне отличное представление о том, что делает моя команда.

С другой стороны, это вызвало у меня самопроизвольный гнев, когда моя смена трех строк заняла 4 часа, потому что мне нужно было отследить другое лидерство и заставить их выполнить коммит. Это подтолкнуло меня к тому, чтобы совершать гораздо менее частые коммиты, чем это рекомендуется, и иногда приводило к проблемам, пытающимся отследить изменения, внесенные разработчиком.

Я бы вообще не рекомендовал его, кроме как в самых нужных условиях. Делать обзоры и коммиты было не так уж плохо на самом деле. Наличие моего собственного процесса, зависящего от прихотей других, хотя приводило в бешенство. Если вы не можете доверять своим разработчикам проверку кода, найдите лучших разработчиков.

Telastyn
источник
8
@HubertKario - если ваши лучшие разработчики тратят время на проверку кода, а остальные эффективно блокируются до тех пор, пока не завершится, я не вижу особых практических различий.
Теластин
6
Как они заблокированы? Вы создаете патч (фиксируете локально), отправляете его в апстрим и продолжаете работать над новым виджетом (создаете новые локальные коммиты). Если ваше изменение применяется дословно, вам просто нужно оформить заказ и объединить репо лидера. Если это не было применено дословно, вы все равно можете отменить свою более позднюю работу. Если изменение действительно критично, вы можете опубликовать его в своем публичном репо и попросить людей оформить его оттуда или просто отправить им исправления. В этом случае gitобнаружит, что изменение уже внесено и просто пропустит применение определенного апстрим-патча.
Юбер Карио
9
Последняя строчка в этом вопросе - действительно весь смысл в моих глазах. Разработчик, которому не доверяют, в лучшем случае будет неэффективен, а в худшем - ненавистен к своей работе. Не нанимайте людей, которым вы не будете доверять. Это бессмысленно тратить деньги на людей, которым вы не позволите выполнять работу, за которую им все равно платят.
Джимми Хоффа
1
@HubertKario - вы знаете лучше, чем я. Окружающая среда, в которой я находился, раздражала манипулирование различными ветками / наборами изменений.
Теластин
5
@Telastyn Я не знаю, должен ли я истолковывать ваш ответ так же буквально, как и я, но еще одним недостатком этого является история аннотаций / обвинений, которая будет неверной. Если вы нашли какой-то код, который вы не понимали, вы бы спросили рецензента, который его совершил, а не программиста, который его написал.
Даниэль Каплан
28

Любой должен быть в состоянии совершить.

Если у вас есть проблемы с исправлением ошибок, это не политика контроля версий, которая является неправильной. Это разработчики, которые не могут убедиться, что то, что он / она совершает, работает. Итак, что вам нужно сделать, это определить четкие рекомендации о том, что делать и когда.

Еще одна замечательная вещь называется юнит-тесты;)

Хотя есть альтернатива.

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

б) В Subversion и аналогичных вы можете использовать ветви, где каждый разработчик должен создавать патчи, чтобы получить его в основную ветку.

jgauffin
источник
1
Это. Если вы выполняете коммиты без модульных и сборочных тестов, требование проверки кода является несовершенной повязкой.
Брайан Кноблаух
Да. Вот почему я упомянул альтернативы. Обзоры кода лучше, чем ничего. Невозможность версии кода - это боль, с которой не должен сталкиваться ни один разработчик.
jgauffin
2
Модульные тесты не помогают, если они написаны теми же <вставьте ваши любимые 4 буквы здесь>, что и код модуля.
ot--
@BrianKnoblauch: можно утверждать, что верно и обратное. В идеале вы должны иметь оба.
Док Браун
@ ott-- Я только что услышал историю о разработчике, который ушел после совершения ужасного беспорядка и исправления всех утверждений в его модульных тестах. По умолчанию тесты проходят успешно, поэтому нужно было заметить проблему!
Алекс
8

Вам следует взглянуть на такие проекты, как Gerrit, который позволяет всем разработчикам помещать свой код в ветку 'review', и как только вы, старшие / ведущие разработчики, довольны этими изменениями, они могут вставить их в master / release.

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

Таким образом, любой, кто ожидает изменения, может получить его, как только он будет готов, и только квалифицированные специалисты (с правами +2 в Gerrit) смогут подтолкнуть этот код для тестирования, а затем и для производства.

rochal
источник
2
Мы используем геррит с большим успехом. Решает каждую проблему, с которой у ОП есть проблема, и даже с некоторыми, о которых он не знает.
Mattnz
8

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

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

  1. Нет - я хочу увидеть каждую строку кода, прежде чем вы его зарегистрируете.
  2. Некоторые - дайте мне знать, что вы делаете, и я оставлю отзыв
  3. Большинство - иди делай свою работу и проси о помощи только тогда, когда это необходимо.

Преимущества высвобождения вашего таланта:

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

Есть разработчики, заинтересованные в пути управления, которые могут предпочесть не кодировать весь день; оставь остальных в покое.

JeffO
источник
1
+1. Пусть команда проверяет команду - и рецензент, и рецензируемый могут получить прибыль, даже если рецензент менее опытен, чем рецензируемый. И вы можете сделать все проверки после регистрации. ИМО, если вы запретите людям регистрироваться, их производительность снизится (несмотря на их мотивацию).
Энди
5

стоит ли обзор производительности?

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

Если все сделано неправильно , снижение производительности, конечно, убьет все преимущества обзора; Ответ заключается не в том, чтобы отбросить идею обзоров, а в том, чтобы понять, как это сделать правильно .

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


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

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

С другой стороны, когда команда разумно сбалансирована, обзоры кода - это весело и познавательно. В моем предыдущем проекте мы требовали 100% проверки кода, и это не занимало много времени и не отвлекало. В ходе обзора были обнаружены ошибки, и были дебаты о стиле кодирования и выборе дизайна, но это было просто ... нормально .


Если изменения кода блокируются в течение нескольких дней ... недель после получения доступа к QA для тестирования "из-за проверок", изучение приемов VCS будет наименее вероятным способом решения этой проблемы. Вместо этого лучше сосредоточить свои усилия на выявлении проблем в том, как организован процесс проверки.

  • - О, интеграция этого изменения была сильно отложена, потому что рецензент внезапно заболел, что за беда.
    - Здравствуйте! Черт возьми, вы когда-нибудь думали о том, чтобы иметь резервных рецензентов, чтобы иметь дело с такими случаями?
комар
источник
4

Да. Но только если вы говорите о распределенном управлении источниками. С централизованным - это зависит.

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

Если программистов очень много, вы можете поручить реальную проверку кода лейтенантам, и ведущий разработчик безоговорочно внесет свои изменения (почти). Это работает для ядра Linux, я не думаю, что есть более крупные программные проекты ...

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

Хуберт Карио
источник
2

Проверка кода не обязательно требует внимания только ваших самых лучших программистов. ИМО, это должна быть неформальная вещь. Просто второе мнение или вторая пара взглядов на фрагмент кода от новичка, прежде чем он будет проверен в производстве. Это помогает смягчить основные упущения, а также помогает людям лучше освоить кодирование как ремесло, открывая для себя другие перспективы разработки.

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

Если бы проверка кода должна была остановить 99,5% ошибок до того, как они попали в вашу базу кода, в действительности не было бы никакого смысла в сложном контроле версий. Тем не менее, git пугает поначалу, но основное общее использование не так сложно, и оно легко настраивается. Вы должны быть в состоянии остановиться на несколько часов, чтобы научить всех, как его использовать. Все совершают. Все, кроме самых заурядных новичков, пересматривают, пока не продемонстрируют опыт в чем-то.

Эрик Реппен
источник
0

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

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

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

Джеймс
источник
0

Да, обзор того стоит. Я не уверен, что производительность снизится, если процесс проверки будет пропорциональным по следующим причинам:

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

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

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

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

Стив
источник
1
-1: Это делает программистов честными - если вы знаете, что они будут рассмотрены, люди будут меньше сокращаться. - Хм ... Я был бы обеспокоен введением морального риска. Таким образом, разработчики могут стать ленивыми или неряшливыми, потому что они знают, что более старший разработчик всегда будет нести ответственность за свой код в форме проверки кода.
Джим Г.
1
Рецензент вообще не несет ответственности за код, а дает советы и инструкции по вопросам, связанным с кодом. Первоначальный разработчик должен решить проблемы и по-прежнему несет ответственность за код.
Стив
-1

Хорошие программисты уходят, если значительная часть их работы заключается в проверке кода других людей?

Если они не получают удовольствия от работы и вынуждены заниматься этим, то ДА. Это очень вероятно случится. Нахождение следующей интересной работы для хорошего разработчика не является большой проблемой в наше время.

Должны ли ваши лучшие программисты проверять чужой код в системе контроля версий?

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

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

Е.Л. Юсубов
источник