Делай как вGoogle
 Скачать 5.77 Mb.
|
|
175 Чтобы оценка правильности была более объективной, предпочтение обычно от- дается авторскому подходу к дизайну или реализации предложенного изменения. Рецензент должен предлагать альтернативы изменения, опираясь не на личное мнение, а на упрощение понимания кода (предлагать менее сложные варианты) или работы с ним (предлагать более эффективные варианты). В целом же инженерам рекомендуется одобрять изменения, улучшающие кодовую базу, даже если они не «идеальны». Это существенно ускоряет обзор кода. По мере развития инструментов все больше проверок правильности будет вы- полняться автоматически, с применением таких методов, как статический анализ и автоматическое тестирование (подробнее о задачах, которые не могут решить инструменты, в главе 20). Несмотря на свои ограничения, инструменты ослабили зависимость проверки правильности кода от человека. Проверка на наличие дефектов на начальном этапе процесса обзора кода является неотъемлемой частью общей политики «сдвига влево», согласно которой мы стре- мимся выявлять ошибки как можно раньше, чтобы на их устранение ушло меньше усилий и ресурсов. Обзор кода не является ни панацеей, ни единственной проверкой правильности — это элемент эшелонированной обороны ПО от проблем в коде. По- этому обзор кода необязательно должен быть «идеальным» по результативности. Как ни удивительно, но проверка правильности — не основное преимущество, кото- рое Google получает от процесса обзора кода. Проверка правильности гарантирует работоспособность изменения, но гораздо важнее, чтобы изменение оставалось по- нятным и осмысленным с течением времени и по мере увеличения кодовой базы. Рассмотрим проверку понятности изменения. Понятность кода Обзор кода обычно является первым взглядом на изменение со стороны. Рецензент делает то, что не может сделать даже самый лучший инженер, — дает непредвзятый отзыв. Обзор кода часто является первой проверкой понятности изменения для бо- лее широкой аудитории. Код читается гораздо чаще, чем пишется, и его восприятие и понимание чрезвычайно важны. Часто полезно найти рецензента, который имеет свой взгляд на изменение, отличный от мнения автора, и, возможно, будет в своей работе поддерживать или использовать предлагаемые изменения. Проверка кода основана как на уважении к мнению его автора, так и на принципе «клиент всегда прав». Какое бы число вопросов автор ни получил сейчас, оно многократно умножится с течением времени. Это не означает, что автор должен изменить свой подход или логику в ответ на критику, но означает, что, возможно, ему нужно объяснить свою позицию более четко. Проверки правильности и понятности кода являются основными критериями для получения LGTM и последующего одобрения. Когда инженер ставит отметку LGTM, он утверждает, что код действует как требуется и он понятен. В Google изменения в коде, который постоянно поддерживается, требуют дополнительного одобрения. 176 Глава 9. Код-ревью Согласованность всей базы кода В конечном счете код, который вы пишете, будет поддерживаться кем-то еще. Многим из тех, кто будет сопровождать ваш код, придется прочитать его и понять, что вы хотели выразить. Другим (в том числе автоматизированным инструментам) может понадобиться выполнить рефакторинг вашего кода, когда вы уже перейдете в другой проект. Поэтому код должен соответствовать стандартам согласованности и не быть чрезмерно сложным. В процессе обзора рецензенты могут оценить, насколько код соответствует стандартам кодовой базы. Таким образом, обзор способствует под- держке работоспособности кода. Именно для простоты сопровождения оценка LGTM (указывающая на правильность и простоту кода для понимания) отделена от оценки удобочитаемости. Одобрения, касающиеся удобочитаемости, могут давать только лица, успешно прошедшие про- цесс обучения удобочитаемости кода на определенном языке программирования. Например, одобрить код на Java может только инженер, получивший сертификат удобочитаемости на Java. Проверяющему удобочитаемость поручается исследовать код и убедиться, что он следует утвержденным передовым практикам для этого конкретного языка программирования, согласуется с базой кода на этом языке в репозитории Google и не слишком сложен. Согласованный и простой код легче понять и проанализи- ровать с применением инструментов, когда придет время для рефакторинга, что делает его более устойчивым. Если определенный паттерн в кодовой базе всегда реализуется каким-то одним способом, это упростит создание инструмента для его рефакторинга. Кроме того, код может быть написан одним человеком, но будет прочитан десят- ками, сотнями или даже тысячами других людей. Согласованность всей базы кода упрощает его понимание всеми инженерами и даже влияет на процесс обзора кода. Иногда она конфликтует с функциональностью: рецензент, оценивающий удобо- читаемость, может выступить за то, чтобы изменение стало менее функциональным, но более простым для понимания. Благодаря согласованности кодовой базы инженерам проще вникать в код других проектов. Для обзора кода инженерам может потребоваться помощь другой команды. Возможность обратиться к экспертам для проверки согласованности кода позволяет инженерам сосредоточиться на правильности и простоте понимания кода. Командная ответственность Обзор кода также дает важные культурные преимущества: он укрепляет инженеров- программистов в мнении, что фактически код является не «их» собственностью, а частью коллективного труда. Такие психологические преимущества могут быть едва заметными, но от этого не менее важными. Если убрать процесс обзора кода, большинство инженеров неизбежно начнут склоняться к индивидуальному стилю Преимущества обзоров кода 177 и подходу в разработке ПО. Процесс обзора кода заставляет автора не только по- зволить другим вносить свой вклад в его творение, но и идти на компромиссы ради общего блага. Человеку свойственно гордиться своим ремеслом и не испытывать особого желания открывать свой код для критики, а также настороженно относиться к критическим отзывам о готовом коде. Процесс обзора кода предусматривает механизм смягчения эмоционального напряжения его автора. Обзор кода не только ставит под сомнение предположения автора, но и делает это предписанным нейтральным образом, чтобы исключить «переход на личности». В конце концов, процесс требует критического обзора (не зря мы назвали наш инструмент для обзора кода «Critique»), поэтому никто не можете обвинить рецензента в том, что он выполняет свою работу и критикует. Таким образом, процесс обзора кода выступает в роли «злого полицейского», тогда как рецензент воспринимается как «добрый полицейский». Конечно, не всем и даже не большинству инженеров нужны такие механизмы смяг- чения. Но создание защиты от такой критики в процессе обзора кода часто дает большинству инженеров гораздо более мягкое представление об ожиданиях команды. Перспектива обзора кода пугает многих инженеров, присоединившихся к Google или перешедших в новую команду. Нетрудно понять, что любая форма критики не- гативно отражается на эффективности работы человека. Но со временем почти все инженеры привыкают, что в процессе обзора их кода возникнут вопросы, и начинают понимать ценность советов и вопросов, предложенных в ходе этого процесса (хотя, по общему признанию, для привыкания требуется время). Еще одним психологическим преимуществом обзора кода является признание его пригодности. Даже самые способные инженеры могут страдать от синдрома самозванца и быть слишком самокритичными. Процесс обзора кода действует как подтверждение и признание их способностей. Часто этот процесс включает обмен идеями и знаниями (подробнее об этом в следующем разделе), что приносит пользу как рецензенту, так и рецензируемому. По мере расширения своих знаний инженерам иногда бывает трудно получить подтверждение своего совершенствования. Процесс обзора кода дает им такую возможность. Процедура инициирования обзора кода также заставляет авторов уделять своим из- менениям больше внимания. Многие инженеры-программисты не являются перфек- ционистами — большинство из них признают, что код, «выполняющий свою работу», лучше, чем совершенный код, разработка которого занимает слишком много времени. В отсутствие процесса обзора кода многие авторы кода начнут срезать углы, чтобы исправить дефекты позже. «Да, у меня нет некоторых юнит-тестов, но я потом их до- бавлю». Обзор кода заставляет инженера решать большинство проблем до отправки изменений. Необходимость собрать все компоненты, составляющие изменение, для обзора кода заставляет инженера убедиться, что код в полном порядке. Короткий период для размышлений перед отправкой изменений — это идеальное время, чтобы еще раз прочитать изменения и убедиться, что ничего не упущено. 178 Глава 9. Код-ревью Обмен знаниями Одним из наиболее важных, но недооцененных преимуществ обзора кода является обмен знаниями. Большинство авторов выбирают рецензентов, которые являются экспертами или, по крайней мере, опытными специалистами в предметной области. Процесс обзора позволяет рецензентам передавать авторам знания, давать советы, рассказывать про новые методы или делиться рекомендациями. (Рецензенты могут помечать некоторые свои комментарии как «FYI» — «на заметку», не требуя каких- либо действий, а просто сообщая автору некоторые сведения.) По мере накопления опыта авторы часто становятся владельцами и затем получают возможность высту- пать в роли рецензентов кода других инженеров. Кроме обратной связи и подтверждения работоспособности кода процесс обзора кода также включает обсуждение реализации изменения, выбранной автором. Не только авторы, но и рецензенты могут узнавать о новых методах и паттернах в процессе обзора. В Google рецензенты могут напрямую делиться возможными вариантами изменений с автором в самом инструменте обзора кода. Инженер может читать не все электронные письма, адресованные ему, но, как пра- вило, он отвечает на каждый комментарий, полученный в процессе обзора кода. Этот обмен знаниями может выходить за рамки часовых поясов и проектов благо- даря средствам быстрого распространения информации среди инженеров Google по всей базе кода. Обзор кода обеспечивает своевременную и эффективную передачу знаний. (Многие инженеры в Google «знакомятся» с другими инженерами при про- хождении обзоров кода!) Время, затрачиваемое на обзоры кода, позволяет инженерам накопить довольно значительные знания. Конечно, основная задача инженеров в Google по-прежнему заключается в программировании, но большую часть своего времени они все еще тратят на обзоры кода. Процесс обзора кода является одним из основных способов взаимодействия инженеров-программистов. Часто в ходе обзора кода демонстрируются новые паттерны, например с помощью рефакторинга в крупномасштабных изменениях. Более того, поскольку каждое изменение становится частью кодовой базы, обзоры кода действуют как хронологические записи. Любой инженер может заглянуть в ко- довую базу Google и определить, когда был принят какой-то паттерн, а также извлечь искомый обзор кода. Часто такие «археологические изыскания» помогают вникнуть в код не только автору и рецензентам, но и гораздо более широкому кругу инженеров. Передовые практики обзора кода Признаем, что обзоры кода могут привести к возникновению трений и задержек. Обычно эти проблемы связаны не с самим обзором, а с выбранной реализацией из- менений. Обеспечение бесперебойности и масштабирования процесса обзора кода в Google требует применения передовых практик. Большинство из этих практик указывают, что обзоры кода должны протекать быстро и гладко. Передовые практики обзора кода 179 Будьте вежливы и профессиональны Как отмечалось в главе 2 «Культура» этой книги, Google активно развивает культуру доверия и уважения. Эта культура избавляет нас от предвзятого взгляда на обзор кода. Например, инженеру-программисту достаточно получить одобрение LGTM только от одного другого инженера, чтобы удовлетворить требование к простоте по- нимания кода. Многие инженеры комментируют и оценивают изменения, понимая, что эти изменения можно внести в кодовую базу без дополнительных проверок. Тем не менее обзоры кода могут вызвать беспокойство и стресс даже у самых способных инженеров. Поэтому крайне важно, чтобы все отзывы и критические замечания не выходили за рамки профессиональной сферы. В общем случае рецензенты уважают выбранные авторами подходы и предлагают альтернативы, только если авторский подход несовершенен. Если автор сумеет продемонстрировать, что несколько подходов одинаково верны, рецензент должен согласиться с предпочтением автора. Даже если в выбранном подходе обнаружатся дефекты, обзор станет платформой для обучения (для обеих сторон!). Все ком- ментарии должны оставаться в строго профессиональных рамках. Рецензенты не должны спешить с выводами относительно особого подхода автора: лучше сначала спросить, почему автор сделал именно такой выбор, прежде чем предполагать, что его подход неверен. Рецензенты должны предоставлять отзывы максимально оперативно. Мы в Google ожидаем, что отзывы поступят в течение 24 (рабочих) часов. Если рецензент не укладывается в это время, ему рекомендуется ответить, что он, по крайней мере, уви- дел изменение и постарается подготовить рецензию как можно скорее. Рецензенты должны избегать рецензирования кода по частям. Ничто так не раздражает автора, как получение отзыва от рецензента, его изучение, а затем получение других отзывов. Мы ожидаем профессионализма не только со стороны рецензента, но и со стороны автора. Помните, что вы — это не ваш код и предложенное вами изменение — не «ваше», а коллективное. В любом случае, как только вы отправите фрагмент кода в репозиторий, он перестанет быть вашим. Будьте готовы объяснить, почему посту- пили именно так, а не иначе. Помните, что понятность кода и простота его поддержки в будущем являются одной из зон ответственности автора. Важно рассматривать каждый комментарий рецензента как элемент TODO («что сделать»). Если вы не согласны с комментарием рецензента, дайте ему знать и опи- шите причину, но не отмечайте комментарий как решенный, пока каждая сторона не получит возможность предложить альтернативу. Один из распространенных способов сохранить корректность дебатов, когда автор не согласен с рецензентом, — предложить альтернативу и попросить другую сторону рассмотреть ее. Помните, что обзор кода — это возможность обучения не только для автора, но и для рецензента. Такой подход часто помогает снизить вероятность разногласий. Точно так же, если вы являетесь владельцем кода и отвечаете за обзор кода в вашей кодовой базе, будьте готовы оценить изменения, предложенные внешним автором. 180 Глава 9. Код-ревью Если изменение направлено на улучшение кодовой базы, вы должны уважительно отнестись к автору, потому что любое такое изменение указывает на то, что в коде еще есть что-то, что можно и нужно улучшить. Пишите небольшие изменения Пожалуй, наиболее важной практикой, обеспечивающей гибкость процесса обзора кода, является внесение в код изменений небольшого объема. В идеале обзор кода должен фокусироваться на одной небольшой проблеме. Процесс обзора кода в Google не предназначен для изменений, оформленных в виде полностью сформированных проектов, и рецензенты вправе отклонить такие изменения как слишком большие для одного обзора. Кроме того, небольшие изменения позволяют инженерам не тратить время на ожидание результатов обзора. Небольшие изменения также имеют свои преимущества в процессе разработки ПО. Определить источник ошибки внутри изменения гораздо проще, если оно достаточно мало. Однако важно понимать, что иногда трудно согласовать процесс обзора кода, опира- ющийся на небольшие изменения, с внедрением новых масштабных особенностей. Набор небольших последовательных изменений проще проверить по отдельности, но труднее оценить в рамках более крупной схемы. Некоторые инженеры в Google, по их признанию, не являются поклонниками практики внесения небольших из- менений. Конечно, существуют методы для управления такими изменениями (раз- работка в интеграционных ветвях, управление изменениями с использованием базы различий, отличной от HEAD), но эти методы влекут существенные накладные расходы. Рассматривайте предложение вносить изменения небольшими порциями только как оптимизацию и допустите в своем коде возможность редкого появления больших изменений. «Небольшие» изменения обычно ограничены примерно 200 строками кода. Неболь- шое изменение должно быть простым для рецензента, чтобы автору не пришлось откладывать другие изменения в ожидании результатов обзора. Большинство из- менений в Google рассматриваются в течение дня 1 . (По крайней мере, первичный отзыв на изменение дается в течение дня.) Около 35 % изменений в Google затраги- вают единственный файл 2 . Чем проще изменение для рецензента, тем быстрее оно попадет в кодовую базу и начнет приносить пользу автору. Ожидание подробного обзора в течение недели или около того может повлиять на последующие изменения. Небольшой первичный обзор предотвращает затраты, которые мог повлечь выбор неправильного подхода. Поскольку изменения передаются обычно небольшими порциями, почти все обзоры кода в Google выполняются одним человеком. В противном случае, если бы прихо- дилось привлекать целую команду, чтобы оценить все изменения в общей кодовой 1 Sadowski C., S öderberg E., Church L., Sipko M., Bacchelli A. Modern code review: a case study at Google ( https://oreil.ly/m7FnJ ). 2 Там же. Передовые практики обзора кода 181 базе, сам процесс оказался бы не масштабируемым. Ограничив размер изменений для обзора, мы оптимизировали этот процесс. Нередки ситуации, когда изменение комментируется несколькими людьми. Часто запрос на обзор направляется одно- му из членов команды, а его копии — соответствующим командам, но основным рецензентом по-прежнему является тот, чья отметка LGTM ожидается, и только одна отметка LGTM необходима для изменения. Любые другие комментарии хотя и важны, но необязательны. Ограничение размера изменений позволяет рецензентам быстро посмотреть, выпол- нил ли основной рецензент свою часть обзора, и сосредоточиться исключительно на особенностях, которые привносит изменение в кодовую базу, а также на сохранности работоспособности кода с течением времени. |