Время, потраченное на обзор кода, потрачено не зря.

Рассмотрим следующий сценарий:

Боб, владелец продукта, ругает команду из трех разработчиков - Элис, Кейт и Дьюка - насчет срочных модификаций, необходимых для отчета, который они только что представили. На следующий день в 10 утра состоится встреча с клиентом, и изменения должны быть внесены, чтобы Боб мог продемонстрировать. Сейчас 18:00. Алиса едет домой, Кейт едет в коттедж (она взяла перерыв). Однако герцог оказывается за своим ноутбуком.

Вот что происходит дальше:

  • Герцог видит сообщение Боба и сообщает Бобу, что без проблем, он может внести изменения. Однако он не будет доступен с 9 вечера и все утро завтра, так как у него есть семейные обязанности. Таким образом, он не сможет продвигать изменения вживую или поддерживать утром.
  • Кейт, будучи пассажиром на заднем сиденье, видит это в Slack и говорит: «Нет проблем, я буду там утром, если что-то нужно закончить».
  • Дюк начинает писать код и открывает запрос на вытягивание (PR).
  • В 20:00 Алиса приходит домой, смотрит на Slack и сообщает команде, что теперь может помочь. Она смотрит на PR Дюка, понимает, что тестов нет, так как это спешная работа, и решает добавить тесты. Поскольку требования предъявляются к Slack, и они могут совместно работать над форками друг друга, это не проблема.
  • Герцог отходит в 21:00.
  • В 22:00 Алиса заканчивает тесты, и к этому времени Кейт уже в сети и может делать обзор. Кейт обнаруживает ошибку (Алиса поменяла некоторые требования в уме, так как она довольно сонная), и после того, как Алиса поправит код, они запускают его вживую.
  • Утром Кейт вносит еще несколько изменений в связи с некоторыми очень дополнительными требованиями от Боба в последнюю минуту и ​​запускает их в действие. Дюк одобряет второй пиар со своего телефона (он волновался и проверял). Встреча с клиентом - это потрясающий успех.

Поменяйте местами пол, и это в основном то, что произошло в моей команде поздно вечером в четверг. Я был очень горд. Как ни странно, это была не та глупая гордость, которую я испытывал, когда мне было 24 года, и всю ночь тащил за стартап: Посмотри на меня, моя работа очень важна, и если я не работаю 24 часа в сутки, весь мир закончится.

На этот раз мою гордость подогревали разные вещи:

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

В результате:

  • Было проведено полное тестирование, хотя это была срочная работа. Таким образом, нет долга разработчикам.
  • Все ошибки вылавливались по отзывам.
  • Командное товарищество и доверие друг к другу были на рекордно высоком уровне!

Я подхожу к году в одной команде, и к этому времени вы уже слишком хорошо знакомы с кодом, поэтому у вас возникают мысли: Могу ли я сменить команду? Теперь есть разные причины для рассмотрения: есть ли у вас проблемы? Что в дорожной карте? Какие у тебя карьерные планы? Я не буду здесь вдаваться в подробности. Я хочу поговорить о том, почему мне особенно нравится работать в этой команде, настолько, что я не хочу переключаться.

Поразмыслив над этим, я понял, что люблю работать в своей команде не из-за того, кто в ней или над чем команда работает. Мне нравится, как мы работаем:

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

Как мы туда попали? Правила и рекомендации по проверке запросов на слияние, которые мы разработали для нашей команды.

Когда команда была сформирована, нас было всего 3 разработчика, владелец продукта и половина менеджера (другая половина нашего менеджера была слишком занята управлением большой командой). Мы решили, что, поскольку нас так мало, мы могли бы действовать как единое целое. Поэтому мы придумали правило проверки PR: Что бы один из нас ни писал, другие 2 должны быть спокойны. Затем, постоянно проверяя PR друг друга, мы расширили набор правил до нескольких руководящие указания.

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

Правило 1

Каждая проверка PR должна иметь как минимум 2 утверждения разработчиками одной и той же группы. Одобрение менеджера не считается.

Первое, что следует отметить, так как мы начинали как команда из 3 человек, это было идеально. Все 3 разработчика были в курсе дела на 100%. С более крупными командами все могло быть иначе. То, к чему вы стремитесь, - это ситуация Дамблдора - крестражи, когда, если вы умрете, как минимум 2 или 3 человека узнают о крестражах. Сейчас в нашей команде 5 разработчиков, из них 2 старожила, 1 новичок и 2 посередине. Мы по-прежнему пытаемся придерживаться модели, при которой все проверяют PR каждого, но, возможно, имеет смысл перейти на 1 старый таймер + минимум 1 другой разработчик.

«Одобрение менеджера не в счет» - может быть, это заставило вас задуматься «ЧТО? Я менеджер и потрясающий программист, и мой обзор имеет значение, как вы смеете ». Обзор менеджера действительно помогает. У нас есть замечательные менеджеры, которые являются отличными программистами, и когда у них есть время на обзор, у них часто есть отличные советы или идеи, чтобы помочь нам.

Но вы должны учитывать, сколько времени ваш менеджер тратит на кодирование. Больше 50%? Тогда они еще и разработчики, так что давайте посчитаем их. Однако в нашей компании менеджеры по разработке очень время от времени занимаются кодированием. В конце концов, именно разработчики должны жить с написанным кодом. Поэтому последнее слово остается за разработчиками. Мне действительно не нравится бесконечный цикл утверждения менеджером-разработчиком, когда менеджер утверждает PR одного разработчика, хотя в команде есть еще 3 разработчика, которые еще не имели возможности взглянуть на него.

Правило 2

У каждого PR должно быть хорошее описание. Прочитав описание, рецензент должен понять, для чего предназначен код. Это должно быть правдой, даже если есть билет Jira или страница требований.

PR без описания - в моей команде это никогда не пройдет. В лучшем случае, они не написали его, потому что в заявке Jira есть хорошее описание. Худший случай: в тикете Jira ничего нет.

Рецензент PR без описания имеет 2 должности:

  1. Понимание того, что делает код, из чтения изменений кода в PR
  2. Пытаетесь решить, выполняет ли код то, что он должен делать, основываясь на ... общем взгляде рецензента на Вселенную и текущую геополитическую обстановку?

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

Правило 3

У PR должен быть достаточный охват модульных и интеграционных тестов.

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

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

Вы также должны подумать, есть ли возможные последствия для производства и требуется ли какое-либо тестирование на живых данных перед развертыванием.

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

Мне лично нравится хранить все это - упоминание о тестах, любых тестах в реальном времени, любых реальных клиентских последствиях / необходимых изменениях конфигурации продукта - в описании PR: так все будет в одном месте. Это может быть связанный билет Jira. Пока это где-то и было учтено.

Правило 4

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

Я думал, что это очевидно, но, как и в случае с большинством очевидных вещей, это не так.

Допустим, вы ошиблись с опечаткой! = Вместо ==. Было 22:00, и вам в любом случае не следовало писать код, но в ту ночь вам хотелось сделать все возможное. Итак, вы открываете PR с исправлением ошибки и меняете! = На ==. Какая-то добрая душа, тоже работающая, одобряет его, и он переходит в ветку master.

Через три дня ваш приятель Герцог, наконец, готов со своим пиаром, который вышел из-под контроля: он подумал, что это небольшая особенность, и не разбил ее на более мелкие билеты, но этого не произошло, и теперь у него есть PR 2000 строк, который он умоляет свою команду пересмотреть.

Duke сливается из восходящего потока, код конфликтует там, где вы заменили == на! =. Герцог устал, он не замечает, что случилось,! = Вернулся.

Поскольку нет теста, чтобы отловить его, ошибка вернулась.

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

Вот и все! Три правила, а четвертое - скорее элемент здравого смысла.

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

Опять же, эти правила могут показаться вам неправильными или могут оказаться полезными. Если они вызвали какие-то мысли или идеи, я рад этому!