Несколько дней назад компания Google выпустила документ с инженерной практикой Google. А содержание документа связано с проверкой кода, в нем содержится информация о том, как инженеры Google проводят проверку кода, а также рекомендации по проверке кода. Оригинальный адрес:Google.GitHub.IO/ Eng -PR act IC ...

Объяснение терминов в этой статье:

cr: code review
cl: список изменений, относится к этому изменению
рецензент: рецензент cr
nit: Полное название — nitpick, что означает «ковырять кости в яйце».
Автор: Это разработчик данного CL.В исходном тексте разработчик называется автором. В образе мышления иностранца это означает "автор CL".

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

стандарт кр

Основная цель CR (просмотр кода) — убедиться, что качество кода кодовой базы Google становится все лучше и лучше. Все сопутствующие инструменты и процессы созданы для этой цели. Для достижения этой цели необходимо сделать ряд компромиссов и компромиссов.

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

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

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

Поэтому у нас есть следующие правила, как стандарт, который мы ожидаем в cr:

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

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

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

гид

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

в общем

Реальность и данные важнее личных предпочтений. Что касается стиля кода, руководство по стилю (style guide) является абсолютным авторитетом. Любые пункты, которых нет в руководстве по стилю (например, пробелы и т. д.), являются вопросом личных предпочтений. Стиль кода должен соответствовать существующим. Если в проекте нет прежнего единого стиля, то примите авторский стиль.

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

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

Решение конфликта

В любом конфликте с cr первым шагом всегда должен быть разработчик и рецензент в соответствии с этой статьей и"Руководство автора CL", попробуйте прийти к консенсусу.

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

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

что смотреть в кр

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

дизайн

В кр важно смотреть на общий дизайн КЛ. Имеет ли смысл взаимодействие разных сегментов кода в CL? Это изменение относится к вашей бизнес-кодовой базе или к другой импортированной кодовой базе? Хорошо ли он интегрируется с остальной системой? Сейчас подходящее время, чтобы добавить эту функцию?

Функция

Делает ли этот CL то, что хотят разработчики? Приносит ли разработчик пользу пользователям, для которых был разработан код? «Пользователи» обычно являются как конечными пользователями (когда на них влияют изменения), так и разработчиками (которые должны будут «использовать» этот код в будущем).

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

Вы можете проверить CL самостоятельно, если хотите. Если изменение будет иметь прямое влияние, видимое пользователем, например изменения пользовательского интерфейса, очень важно проверить изменения CL. При чтении кода бывает сложно понять, как те или иные изменения повлияют на пользователя. Для такого изменения, если не удобно тестировать самостоятельно, можно попросить разработчика продемонстрировать функцию (демо).

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

Сложность

Является ли CL более сложной, чем должна быть? CL для любого уровня должен подтверждать следующие пункты: Не слишком ли сложна одна строка кода? Функция слишком сложная? Класс слишком сложный? «Сложный» обычно означает, что код трудно читать, и, вероятно, также означает, что другие разработчики добавили другие ошибки при изменении кода.

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

Дональд Кнут сказал: Преждевременная оптимизация — корень всех зол

тестовое задание

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

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

Не проходит ли тест, когда код действительно глючит? Будет ли тест генерировать ложные срабатывания, если тестируемая программа будет изменена? Делает ли каждый тест простые и полезные утверждения? Правильно ли разделены тесты между различными методами тестирования?

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

имя

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

Рекомендуемые справочные статьи«Чистый код 101 — Значимые имена и функции».

Примечания

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

Часто комментарии полезны для объяснения того, почему код существует, а не для объяснения того, что делает фрагмент кода. Если сам код нельзя объяснить внятно, значит, его нужно упростить еще больше. Конечно, есть исключения, например, при объяснении того, что делает регулярное выражение или сложный алгоритм, весьма полезен комментарий, объясняющий, что делает код. Но для большинства комментариев он используется для объяснения информации, которая не включена в саму программу, а является информацией, например, почему это сделано именно так.

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

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

стиль

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

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

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

Документация

Если CL изменяется при сборке, тестировании, взаимодействии и публикации, убедитесь, что соответствующая документация также обновлена, включая README, страницы g3doc и другие сгенерированные справочные файлы. Если CL удаляет какой-то код или объявляет его устаревшим, подумайте, следует ли удалить соответствующую документацию, и спросите, не отсутствует ли документация.

каждая строка кода

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

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

Если вы понимаете, но чувствуете, что не имеете права рецензировать раздел, убедитесь, что среди рецензентов есть подходящее (квалифицированное) лицо для рецензирования раздела. Особенно для сложных вопросов, таких как безопасность, параллелизм, доступность, интернационализация и т. д.

контекст

Часто бывает полезно взглянуть на CL в достаточном контексте. Как правило, инструмент cr отображает только несколько строк кода вокруг измененной части. Но иногда вам нужно просмотреть весь файл, чтобы убедиться, что изменения разумны. Например, вы можете увидеть только 4 новые строки кода, но когда вы на самом деле посмотрите на весь файл, вы увидите, что эти 4 строки добавляются к 50 строкам кода, которые необходимо разбить на более мелкие функции.

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

преимущество

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

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

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

Не скупитесь на комплименты сейчас!

Суммировать

При съеживании обязательно:

Код хорошо разработан
Функциональность хороша для пользователей
Будьте разумны и привлекательны для любых изменений пользовательского интерфейса
Любая реализация параллельного программирования безопасна
Код не должен быть сложнее, чем нужно
Разработчики не должны реализовывать функцию, которая сейчас не нужна, но может понадобиться в будущем.
Код имеет правильные модульные тесты
Проверено и хорошо спроектировано
Разработчики используют четкие, недвусмысленные имена для всего
Комментарии должны быть четкими и полезными и объяснять только почему, а не что.
Код имеет надлежащую письменную документацию (обычно в g3doc)
Стиль кода соответствует руководству по стилю.
Обязательно просматривайте каждую строку кода, которую вас просят просмотреть, подтверждайте контекст, убедитесь, что вы улучшаете качество кода, и хвалите разработчиков за хорошее и хорошее!

Как просматривать CL

Теперь вы знаете, на что обращать внимание при просмотре, но какой самый эффективный способ просмотра изменений, разбросанных по нескольким файлам?

Является ли изменение разумным? Есть ли у него хорошее описание
Сначала рассмотрим самые важные изменения в CL. Хорошо ли он спроектирован в целом?
Посмотрите на оставшиеся изменения в CL в разумном порядке.

Шаг 1. Рассмотрите макрос изменения, посмотрите описание CL и то, что оно делает.

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

Например, вы можете сказать: «Похоже, вы проделали хорошую работу, спасибо! Но на самом деле мы движемся к удалению системы FooWidget, которую вы модифицируете, поэтому мы не хотим вносить в нее какие-либо новые модификации в ближайшее время. на этом этапе. Как насчет рефакторинга нашего нового класса BarWidget?»

Важно отметить, что рецензент вежливо отклоняет CL, предлагая альтернативы, и при этом остается вежливым. Эта вежливость важна, потому что мы хотим проявлять взаимное уважение, даже если мы не согласны.

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

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

Укажите на проблемы, сообщите об альтернативах или дайте указания

Шаг 2: Проверьте основные части CL

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

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

Есть две основные причины, по которым важно сразу же присылать комментарии по основным проектам:

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

Шаг 3. Просмотрите остальные изменения CL в разумном порядке.

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

скорость просмотра

Почему обзор быстрее

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

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

Насколько быстрым должен быть cr?

Если вы не нуждаетесь в своем бизнесе, то вам следует выполнить ОБЗОР, как только вы поданы. В ОТЗЫВЕ ответ - самый длинный лимит - рабочий день. Если вы следуете приведенным выше рекомендациям, это означает, что общий CL должен пройти несколько раундов ПРОВЕРКИ (при необходимости).

Скорость против прерывания

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

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

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

быстрый ответ

Когда мы говорим о скорости CR, мы фокусируемся на времени отклика, а не на том, сколько времени требуется CL для завершения и фиксации. В идеале весь процесс должен быть быстрым.

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

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

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

Тем не менее, время отклика идеального человека максимально быстрое.

Обзор в разных часовых поясах

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

ЛГТМ Отзывы

Чтобы ускорить процесс, в некоторых случаях рецензенты могут дать LGTM или одобрение, даже если на CL все еще есть нерешенные рецензии. Аналогичная ситуация возникает в:

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

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

Большие перемены

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

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

Способность CR будет улучшаться со временем

Если вы будете следовать этим рекомендациям и будете очень строго относиться к cr, вы обнаружите, что весь процесс cr будет становиться все быстрее и быстрее. Потому что разработчики узнают, что такое качественный код, и с самого начала стремятся к отличному CL, который просто занимает все меньше и меньше времени. Рецензенты, с другой стороны, учатся быстро реагировать, а не добавлять ненужные задержки в процесс. Но не идите на компромисс со стандартами CR и качеством кода ради воображаемой скорости, в конце концов, в конечном итоге это ничего не ускорит.

чрезвычайное происшествие

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

Как написать рецензию рецензию

Как бороться с отсроченным обзором

Иногда разработчики задерживают обработку комментариев, сгенерированных cr. Либо они не согласятся с вашим советом, либо будут жаловаться, что вы слишком строги.

кто прав

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

Но и разработчики не всегда правы. В этом случае рецензент должен пойти дальше и объяснить, почему он считает свое предложение правильным. Хорошее объяснение обычно показывает «понимание ответа разработчика» и информацию о том, «почему было запрошено изменение». В частности, рецензенты должны продолжать продвигать свои аргументы, когда они чувствуют, что высказанные предложения улучшат качество кода. Пока они считают, что требуемые дополнительные усилия в конечном итоге улучшат общее качество кода. Улучшение общего качества кода часто происходит с каждым крошечным изменением. Иногда требуется несколько объяснений предложения, чтобы другой человек действительно понял ваши намерения. Помните, всегда будьте вежливы и дайте понять разработчику, что вы слышали, что они говорили, и вы просто не согласны с аргументом.

разочаровывающие разработчики

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

Очистить позже

Распространенной причиной задержки является то, что разработчик надеется выполнить задачу (по понятным причинам). Они не хотят утверждать это очередным раундом CL кр. В этот момент обычно говорят, что после завершения CL, так что теперь вы должны дать LGTM. Конечно, некоторые разработчики очень хороши в этом и сразу же рассылают последующий CL для устранения проблемы (последующий CL), но исходя из прошлого опыта, такое последующее действие для «очистки» встречается очень редко. Если только разработчик не осуществил после получения одобрения CL немедленные действия по очистке, иначе таких вещей никогда не произойдет. Это не потому, что разработчики безответственны, а потому, что у них может быть много другой работы, которую нужно выполнить, поэтому работа по очистке будет забыта в виде кучи работы. Поэтому разработчикам обычно лучше придерживаться их, чтобы очистить их после слияния в коде. Поскольку они позволяют людям «почистить позже», качество кодовой базы приводит к ухудшению в наиболее распространенной ситуации.

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

Распространенные жалобы на строгость проверки

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

Решение конфликта

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

Резюме переводчика

стандарт кр

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

что смотреть в кр

Общий дизайн КЛ
Функциональная проверка, функциональность хороша для пользователей, разумна и красива для любых изменений пользовательского интерфейса.
Это очень сложно и чрезмерно?
Код имеет правильные модульные тесты
Проверено и хорошо спроектировано
Какая спецификация именования, см. имя, чтобы знать
Соответствующие комментарии, комментарии должны быть почему не что
Стиль кода соответствует руководству по стилю, если вам нужно изменить стиль кода, это должно быть решено в другом CL.
Обновлять документацию при изменении сборки, тестирования, взаимодействия и выпуска CL.
Внимательно просмотрите каждую строку кода (кроме файлов ресурсов, сгенерированного кода, больших структур данных). Если сложнее, пусть разработчик объяснит
Нужны более подходящие люди для рассмотрения сложных вопросов, таких как безопасность, параллелизм, доступность, интернационализация и т. д.
Думайте о CL как о целостной системе
Когда ОБЗОР, я сказал разработчику сделать что-то не так, лучше сказать им, что происходит?

Как просматривать CL

Макрос представления изменения см. в описании CL и его функциях.
Проверьте основную часть CL
См. остальные изменения CL в разумном порядке.

скорость просмотра

Медленная скорость ревью приведет к снижению общей скорости работы команды, разработчики начнут протестовать cr, пострадает качество кода
Если вы находитесь в то время, когда вам нужно сосредоточиться на работе (скажем, на написании кода), не прерывайте себя, чтобы выполнить контрольную работу.
Скорость, с которой человек отвечает на комментарии, важнее, чем быстрое завершение всего процесса CR.
Столкнувшись с другим часовым поясом, попробуйте ответить автору, пока он еще в офисе.
Чтобы ускорить процесс, в некоторых случаях рецензенты могут давать LGTM или одобрение, даже если на CL все еще есть нерешенные отзывы.
Когда ревью затруднена из-за слишком больших изменений, обычно просят разработчика разобрать CL на несколько меньших CL.
Скорость CR должна быть все быстрее и быстрее, но не ставьте под угрозу стандарты CR и качество кода, чтобы улучшить воображаемую скорость.

Как написать рецензию рецензию

Когда разработчик не согласен с вашим предложением, подумайте о том, кто прав, объясняет четко, и будь вежливым.
Если обзор будет настаивать на своем собственном мнении, это расстроит разработчика. Разочарование часто связано с тем, как написаны обзоры CR, а не с тем, что рецензент настаивает на качестве кода.
Если CL представляет собой новую проблему, ее необходимо устранить до фиксации, если только это не чрезвычайная ситуация.
TODO комментарии и ссылка на только что зарегистрированную ошибку, если проблема с комментарием обзора не может быть решена сейчас.

Что делать, если отзыв слишком строгий и пожаловался

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

Обратите внимание на официальный аккаунт «Другой интерфейс», изучите интерфейс с другой точки зрения, быстро растем, играйте в новейшие технологии и исследуйте различные черные технологии вместе.