В первая часть В материале были рассмотрены аспекты стандартов code review и моменты, на которые необходимо обратить внимание в первую очередь.
В заключительной части мы поговорим о:
- процедура рассмотрения,
- скорость (и на что она влияет),
- как правильно писать комментарии,
- обсуждения во время обзора.
ТЛ;ДР
Теперь, когда это ясно на что нужно обратить внимание , вам необходимо решить, какова процедура рассмотрения.Существует 3 основных этапа:
- Понять, нужен ли представленный функционал в принципе и есть ли у него хорошие особенности описание ;
- Обратите внимание на то, что самое важное в измененном коде и насколько хорошо спроектировано решение в целом;
- Просмотрите оставшиеся изменения в том порядке, в котором вы считаете нужным.
1.1. Обзор изменений
Посмотри на Описание КЛ и его основной функционал: нужны ли эти изменения? Если предлагаемый функционал не нужен, постарайтесь как можно скорее сообщить об этом и объяснить причины.В этом случае целесообразно предложить разработчику задание взамен.
Ваше сообщение может выглядеть так: «Спасибо за хороший код, однако сейчас мы движемся к полному избавлению от системы FooWidget, которая здесь затронута.
Поэтому в настоящее время мы не модифицируем ее каким-либо образом.
здорово, если вы помогли с рефакторингом класса BarWidget. Что вы думаете?" Обратите внимание на общий тон сообщения рецензента: он вежливо и вежливо отверг КЛ и предложил альтернативу.
Такого рода такт очень важен: так мы проявляем уважение к другим разработчикам, даже если между нами есть разногласия.
Если такие ситуации возникают часто и вы получаете много запросов на проверку изменений, которые вы считаете ненужными в вашей системе, то задумайтесь, как устроен процесс внесения изменений в ваш код для внешних разработчиков — возможно, вам потребуется утвердить изменения перед написанием кода.
Лучше сказать «нет» до того, как человек проделает большую работу, которая в конечном итоге окажется ненужной.
1.2. Проверка самого важного
Найдите самую важную часть CL — часто это единственный файл, в котором изменилась больше всего логика.Сначала протестируйте важные изменения — это даст вам необходимый контекст для тестирования всего остального.
Если CL слишком большой и вы не можете выделить основные части, спросите у самого разработчика, что здесь самое важное, или спросите его разбить CL на несколько более мелких .
Если вы видите принципиальные проблемы с CL, пожалуйста, оставьте комментарий об этом как можно скорее, даже если у вас нет времени смотреть CL до конца прямо сейчас.
Возможно, учитывая выявленные вами проблемы, смотреть оставшиеся изменения нет смысла - они все равно будут переписаны.
Почему важно сообщать о серьезных проблемах заранее:
- Часто разработчики отправляют CL на проверку, а затем сразу начинают работу на основе только что написанного кода.
Чем раньше вы сообщите об обнаруженных важных проблемах, тем меньше работы разработчику придется переделывать.
- Серьезные проблемы сложнее, и их решение занимает больше времени.
Практически у всех разработчиков есть дедлайны, и если мы хотим иметь качественную кодовую базу и при этом укладываться в сроки, то лучше исправлять большие проблемы как можно скорее, когда до дедлайна еще далеко.
1.3. Проверяем все остальное
Когда вы поняли, что серьезных проблем в CL нет, нужно проверить остальной код. Выберите порядок проверки самостоятельно или просто примите вариант, предложенный вашим инструментом проверки.Иногда полезно сначала просмотреть тесты, чтобы понять, что должен делать код. Главное — убедиться, что вы проверяете каждый файл.
2. Скорость рассмотрения
2.1. Почему обзоры кода следует проводить быстро?
В Google мы стараемся оптимизировать скорость, с которой команда разработчиков сотрудничает при создании продукта, а не оптимизировать скорость, с которой каждый отдельный разработчик пишет код. Скорость разработчика, конечно, важна, но важнее скорость команды в целом.Что происходит, когда проверки кода выполняются медленно:
- Производительность команды снижается.
Конечно, разработчики, не получившие обзор вовремя, просто начинают заниматься другой работой.В то же время выпуск исправлений ошибок и новых функций откладывается на дни, недели и месяцы, поскольку изменения ожидают рассмотрения и повторного рассмотрения.
- Разработчики недовольны процессом проверки.
Если рецензент отвечает раз в несколько дней и каждый раз просит внести серьезные изменения, разработчикам становится сложно работать.Часто это выражается в жалобах на строгость рецензента.
Однако, если рецензент постоянно просит одинаковый значительные изменения, улучшающие кодовую базу, при этом CL быстро реагирует на обновления, жалобы уходят. Большинство жалоб на процесс рассмотрения исчезает, когда он становится быстрым.
- Негативное влияние на состояние кодовой базы.
Когда проверки проходят медленно, давление на команду возрастает, и код низкого качества часто игнорируется.Медленные проверки также затрудняют рефакторинг, удаление избыточного кода и другие улучшения CL.
2.2. Насколько быстрыми должны быть обзоры?
Если вы не находитесь в процессе целенаправленной работы над задачей, то вам следует сделать обзор, как только его у вас потребуют. Максимальное время ответа на запрос на проверку — один рабочий день.(по сути, именно здесь вам следует начать следующее рабочее утро, если вы не ответили за текущий день).
Если вы будете следовать этим правилам, то обычный CL пройдет несколько раундов проверки (при необходимости) в течение одного рабочего дня.
2.3. Скорость против прерывания
Бывает ситуация, когда отзыв может подождать: Если вы сосредоточенно работаете над задачей, например, пишете код, то не стоит сразу отвлекаться на проведение ревью.Исследования показывают, что разработчику требуется много времени, чтобы вернуться к работе после того, как он отвлекся.
Таким образом, ваш быстрый ответ на отзыв в конечном итоге обходится команде дороже, чем короткое ожидание другого инженера.
Хорошие моменты для проведения обзора могут быть связаны с перерывами в вашей работе: время после выполнения текущей задачи, возвращение из кофе-пойнта, встречи, обеда и т. д.
2.4. Ответить быстро
Когда мы говорим о скорости код-ревью, мы имеем в виду, в первую очередь, время отклика, а не сколько времени вообще нужно CL, чтобы пройти все этапы и закоммититься.Общее время всей проверки также должно быть небольшим, но гораздо важнее быстрая реакция на каждый отдельный запрос.
Иногда сам процесс проверки занимает много времени, в таких случаях быстрая обратная связь существенно меняет впечатление разработчиков о том, что процесс идет медленно.
Если вы слишком заняты, чтобы провести полную проверку, вы можете сообщить разработчику, когда сможете поработать над его CL, или порекомендовать других рецензентов, которые смогут ответить быстрее.
Вы также можете оставить сначала просмотрите комментарии к коду .
Но помните, даже такие ответы не должны прерывать вашу работу по кодированию — лучше отвечать во время перерыва.
Очень важно, чтобы рецензенты тратили на рецензирование столько времени, чтобы с уверенностью сказать, что код соответствует наши стандарты .
Однако в идеале ответы все равно должны оставаться быстрый .
2.5. Разные часовые пояса
Если человек, чей код вы просматриваете, находится в другом часовом поясе, постарайтесь ответить до того, как он уйдет с работы.В противном случае будьте уверены, что проверка будет сделана до начала его следующего рабочего дня.
2.6. LGTM (мне кажется хорошо = с моей точки зрения все хорошо) с комментариями
В некоторых ситуациях, чтобы ускорить проверку, вы можете одобрить ее, даже если в CL еще есть комментарии.Это можно сделать в следующих случаях:
- Рецензент уверен, что разработчик внесет исправления в код в соответствии с замечаниями.
- Остались мелкие изменения, которые не нужны.
2.6. Большой CL
Если к вам на рассмотрение приходит крупный CL и вы не уверены, что у вас достаточно времени, вашей обычной реакцией может быть вопрос: разделить CL на несколько более мелких CL , в зависимости друг от друга.Обычно это возможно и очень помогает рецензентам, хотя и требует дополнительных усилий со стороны разработчика.
Если CL невозможно разбить на более мелкие части и у вас нет достаточно времени для проведения полного обзора, напишите несколько комментариев по общей архитектуре CL, возможно, есть моменты, которые разработчик может улучшить уже сейчас.
Одна из ваших задач как рецензента — дать разработчикам быстрый ответ, а не блокировать их дальнейшую работу.
2.7. Прогресс в проверке кода
Если вы будете следовать всем перечисленным выше правилам и достаточно строги в своих отзывах, вы обнаружите, что весь процесс с каждым разом занимает все меньше и меньше времени.Разработчики понимают, что нужно учитывать при написании кода, и с первого раза присылают вам хорошие CL. Рецензенты учатся реагировать быстро и не замедлять процесс.
Однако, никогда не идти на компромисс, когда дело доходит до стандарты кодирования и качество - несмотря на кажущуюся выгоду делать все быстро сейчас, вы усугубите ситуацию на расстоянии.
2.8. Эаварийные ситуации
Иногда есть аварийные ситуации , когда CL необходимо очень быстро пройти весь процесс проверки.В таких случаях стандарты качества могут быть снижены.
Чтобы понять, какие ситуации являются аварийными, см.
главу "Эчрезвычайные ситуации" .
3. Как писать комментарии в отзывах
ТЛ;ДР
- Будь добрым.
- Объясните свои причины.
- Найдите баланс между прямыми инструкциями и простым указанием на проблемы и предоставлением разработчику возможности решить их самостоятельно.
- Мотивируйте разработчиков писать простой код и добавлять комментарии, вместо того, чтобы объяснять сложные области конкретно вам.
3.1. Вежливость
Ваши комментарии должны быть вежливыми, тактичными и в то же время полезными и понятными для разработчика, код которого вы просматриваете.Один из способов добиться этого — убедиться, что вы пишете комментарии о коде, а не о его авторе.
Не обязательно слепо следовать этому правилу, но его определенно стоит использовать, если вы хотите написать что-то, что может кого-то обидеть или показаться придиркой.
Например: Плохо: «Почему Ты решили использовать потоки, несмотря на то, что многопоточность здесь никакой пользы не дает? Хорошо: «В данном случае многопоточность не приносит никакой пользы, а только усложняет систему.
Поскольку прироста производительности нет, лучше запускать этот код в одном потоке».
3.2. Объясните причины
Выше в примере «Окей» есть объяснение, почему вы оставляете этот комментарий.Это не всегда необходимо делать, но иногда стоит предоставить немного больше информации — предоставить ссылку на передовую практику, которую вы имеете в виду, или объяснить, как ваше предложение улучшит базу кода.
3.3. Дайте нам несколько советов
Внесение изменений в КЛ не является обязанностью рецензента, это обязанность разработчика..
Вам не обязательно предоставлять подробное решение в комментариях и писать код для разработчика.
Это не значит, что рецензент не должен помогать.
В общем, вам следует найти приемлемый баланс между простым указанием на проблему и написанием прямых инструкций по ее устранению.
С одной стороны, разработчик научится быстрее, если будет принимать решения самостоятельно и, кроме того, он ближе к коду, чем рецензент, а значит, как правило, может предложить лучшее решение.
С другой стороны, иногда полезнее давать прямые указания.
Стоит помнить, что основная цель обзора — довести CL до достаточно хорошего состояния; прогресс разработчика — второстепенная цель, хотя тоже важная.
3.4. Следуйте объяснениям
Если вы просите разработчика объяснить какую-то часть кода, которую вы не понимаете, то чаще всего это означает, что код следует переписать в более простом виде .Иногда правильным ответом на такой запрос будет добавление комментария, но помните, что комментарий должен не просто объяснять, почему код такой сложный, в этом случае он должен объяснять смысл самого кода.
Пояснения, написанные только в инструменте обзора, бесполезны для будущих читателей.
Они уместны только в определенных ситуациях: например, когда вы не очень хорошо знакомы с измененным участком кода, но другие, обычные читатели кода, его хорошо знают. 4. Обсуждения в процессе проверки Иногда разработчики не принимают ваши комментарии к обзору, просто не соглашаясь или жалуясь, что вы слишком строги.
4.1. Кто прав?
Если разработчик не согласен с вашим комментарием к его коду, начните с двойной проверки, чтобы убедиться, что он прав.Очень часто разработчик ближе к коду, чем вы, и некоторые детали ему известны лучше.
Насколько обоснованы его аргументы? Насколько они значимы с точки зрения развития кодовой базы в будущем? Если разработчик прав, дайте ему знать и закройте вопрос.
Однако автор кода не всегда прав.
В этом случае рецензенту следует более подробно объяснить, почему он считает свой комментарий значимым.
Хороший ответ демонстрирует понимание позиции разработчика и дополнительную информацию о том, почему комментарий действителен.
Если рецензент считает, что его предложение улучшит состояние кодовой базы и затраченные усилия будут оправданы, ему следует последовательно настаивать на изменениях.
Улучшение кодовой базы обычно происходит небольшими шагами.
.
Иногда требуется несколько раундов переговоров, чтобы убедить застройщика.
Всегда помни, что ты должен остаться вежливый .
Сообщите об этом разработчику вы понимаете его аргументы, а просто не согласен с ними.
4.2. Разработчики расстроены?
Иногда рецензенты полагают, что разработчик может расстроиться или обидеться, если он продолжит настаивать на изменениях.Иногда такое случается, но обычно это временно и в конце концов разработчик обычно благодарен, что вы смогли улучшить предложенный им код. Обычно, если вы вежливый в своих комментариях разработчики ничуть не расстраиваются, а все обиды надуманы рецензентом.
Весь негатив обычно больше про Как комментарии пишутся, а не по факту запроса изменений.
4.3. "Я сделаю это позже"
Разработчики хотят, чтобы все было сделано (и это разумно), и часто не хотят проходить еще один раунд проверок.Поэтому типичные ответы на ваши комментарии будут связаны с обещаниями что-то исправить в следующем CL и пожеланием, чтобы вы одобрили изменения уже сейчас.
Некоторые разработчики добросовестно держат свое слово и оперативно делают новый CL со всеми исправлениями.
Однако, как показывает практика, чем больше времени пройдет с момента создания CL, тем меньше вероятность того, что дополнительные исправления когда-либо будут внесены.
И тут дело не в лени или безответственности, просто у разработчиков много работы и они могли забыть о своем обещании.
Обычно лучший способ добиться изменений — внести их сразу, до того, как код будет объединен в общую кодовую базу и задача станет «выполненной».
Позволяя откладывать исправления на потом, вы серьезно ухудшаете состояние кодовой базы проекта.
Если CL вносит ненужную сложность в код, то CL следует исправлять только в том случае, если это не так.
Если при работе над CL были обнаружены дополнительные проблемы, которые сейчас не могут быть решены, то разработчику следует создать ошибку для исправления и при желании добавить в код комментарий TODO со ссылкой на ошибку.
4.4. Жалобы на строгость отзывов
Если раньше ваши отзывы были слабыми, а теперь вы стали более требовательны, некоторые разработчики будут очень недовольны.Достаточный скорость обзора обычно сводит на нет такие жалобы.
Иногда требуются месяцы, чтобы все недоразумения полностью исчезли, но в целом разработчики начинают понимать ценность строгих проверок, когда видят, как улучшается их код. Даже самые ярые противники строгих проверок могут стать вашими лучшими союзниками, когда они столкнутся с практической ситуацией, доказывающей, что ваша решительная позиция при проверке была правильной.
4.5. Решение конфликта
Если вы соблюдаете все вышеперечисленные правила, но конфликты с разработчиками все равно возникают и вы не можете их разрешить, обратитесь в Стандарты проверки кода понять практики и стандарты, которые могут помочь вам разрешить конфликт. Теги: #программирование #Управление разработкой #перевод #перевод с английского-
Мечты Гейши
19 Oct, 24 -
Факты О Беспроводных Сетях
19 Oct, 24 -
Wpa3 Мог Бы Быть Безопаснее: Мнение Эксперта
19 Oct, 24 -
Internet Explorer 9: Первые Подробности
19 Oct, 24 -
Стивен Спилберг Увлекается Видеоиграми
19 Oct, 24
