Введение

Я был вдохновлен на написание этой мини-статьи из-за кода с более чем 200 запутанными строковыми функциями и бесчисленными операторами if-else, где любое «else» могло, казалось бы, переопределить значение, присвоенное в предыдущем if-else (да, не только if-else, но если, если, если, если, если…) и резюмировать то, что я считаю одним из наиболее важных моментов проверки кода и, надеюсь, поможет задать настроение и путь к более качественному программному обеспечению для разработчиков. Может быть, вы не прочтете ничего, о чем хотя бы подсознательно не знали. Но вот мои 5 центов в любом случае - компиляция моего мнения, которое было более или менее подкреплено обсуждениями, которые у меня были в прошлом с другими разработчиками.

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

Причины проведения проверки кода

  1. Повышение качества кода — удобство сопровождения, читабельность, что должно сократить время производства. Таким образом, в основном аргумент «слишком много работы и нет времени на обзоры кода» (я слышал это слишком часто) и рефакторинг теряет свою актуальность «в долгосрочной перспективе»;
  2. Проверка кода иногда может даже помочь выявить ошибки (уменьшить риск) до того, как они будут развернуты, особенно если их просматривает разработчик, который не разрабатывал эту функцию и может иметь свежий взгляд. Это приводит к меньшему обмену мнениями между разработчиками и теми, кто занимается контролем качества или случайно находит ошибки;
  3. обмен опытом;
  4. Улучшение навыков разработки;
  5. Знакомство с кодовой базой;
  6. Улучшение сплоченности команды — поощрение разработчиков к обсуждению передового опыта друг с другом;
  7. Меньше ошибок = улучшенный путь пользователя к продуктам;

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

Как создавать удобные для просмотра коммиты

  1. Разбивайте большие и сложные коммиты на более мелкие, которые содержат только 1 обновление, новую функцию или исправление (если только это 1 обновление не вынуждает нас вносить другие изменения, которые затем должны быть упомянуты в сообщении коммита; в таком случае, вероятно, необходимо иметь их все в одном коммите). Трудно просматривать коммиты, в которых десятки разных файлов изменены разными видами изменений.
  2. При коммите разработчики должны добавлять «нормальный» заголовок и описание, а не просто все время писать то «обновление», то «исправления», то «новые фичи», в этом каждый может разобраться, не заглядывая в лог истории. Описание должно охватывать что было изменено, почему это было изменено и как была решена проблема (конечно, "как" не всегда требуется, например, при внесении небольшого изменения, такого как изменение цвета кнопки).

Например, некоторые сообщения фиксации можно написать:

  • «Запрос тимлида [ПОЧЕМУ]: изменить положение кнопки CTA [ЧТО]»;
  • «Билет 1887›››[ПОЧЕМУ]: улучшенная производительность››› [ЧТО] за счет реализации отложенной загрузки Datagrid›››[КАК]»;
  • «Обнаружена ошибка [ПОЧЕМУ]: невозможно было нажать кнопку из-за неправильного наложения слоев [ПОЧЕМУ]»;

Это упростит и ускорит рецензирование кода и предоставление отзывов.

«Когда» следует проводить код-ревью

Просмотрите код, прежде чем он покинет ваш компьютер

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

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

Проведите рецензирование кода перед контролем качества или развертыванием

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

Каждый член команды должен проверять код («на мой взгляд»)

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

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

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

Стиль кодирования

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

Быстрый вывод на рынок или чистый код

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

"Когда давление высокое, хочется забыть, что качество, достигнутое в результате проверки кода, имеет дополнительные преимущества, которые ускоряют разработку". по https://www.pullrequest.com/blog/5-steps-to-an-efficient-code-review-culture/

Если я перефразирую это предложение, я бы сказал: «Когда давление очень велико (потому что, скорее всего, на кону выручка) и не хватает времени для правильной реализации некоторых функций, можно запускать код низкого качества в производство до тех пор, пока поскольку он протестирован, и люди понимают, что нам, разработчикам, придется потом потратить больше времени на рефакторинг и «снова» повторно протестировать рефакторинговый код. Но, как всегда, это зависит от… Мы должны руководствоваться здравым смыслом и оценивать, стоит ли тратить свои усилия на рефакторинг… Иногда есть функции, которые вы реализуете, а затем, по сути, никогда не трогаете их снова — тогда можно оставить код более низкого качества и сделать что-нибудь другое. рефакторинг только в том случае, если вам когда-нибудь придется коснуться его снова и следовать принципу «оставь свой код чище, чем ты его получил». Опять же, просто руководствуйтесь здравым смыслом и оцените необходимое время и усилия по сравнению с фактической ценностью, размером кода, риском регрессии и т. д.

Согласно некоторым исследованиям, инженеры тратят почти половину своего времени на технический долг и техническое обслуживание. По своему личному опыту могу сказать, что это может быть правдой. Я сам потратил много времени в своей карьере на рефакторинг кода, который страдал от проблем с читаемостью, хрупкостью (т. е. проблем с ремонтопригодностью — вы исправляете 1 вещь и ломаете 5 других) и т. д., и да, часть этого кода была моей.

Есть в основном 2 вида технического долга и технического обслуживания:

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

номер 2 — это то, что мы хотим максимально минимизировать.

Вот какая-то статья, которую я нашел о мировых затратах на работу с устаревшим и плохим программным обеспечением (хотя не уверен, что все детали верны, но я не удивлюсь, если это так): https://www.cnbc.com/ 06.09.2018/companies-worry-more-about-access-to-software-developers-than-capital.html

Иногда легче сказать, чем сделать

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

Возможно, ваш код никогда не будет идеальным

Некоторые вещи являются либо просто вопросом предпочтений (например, фигурная скобка после оператора if в той же или следующей строке и т. д.), либо/и компромиссами: удобочитаемость и размер, память и производительность, ремонтопригодность и удобочитаемость, требования. по сравнению с осуществимостью и т. д. и т. д.

Поощрение обсуждений

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

  • Почему мы меняем это?
  • Есть ли лучшие решения?
  • Какие еще решения вы рассматривали?
  • С какими проблемами вы столкнулись?
  • Есть ли область кода, в которой вы действительно не уверены, на которой вам действительно хотелось бы уделить больше внимания?

Вещи, которые нельзя пропустить и игнорировать

  1. Некоторые люди могут бояться давать или получать обратную связь;
  2. Важно дать разработчикам понять, что мы проводим проверки кода, чтобы помочь друг другу улучшить свои навыки и знания;
  3. Важно понимать, что вы не всегда правы и должны быть открыты для новых предложений, независимо от того, сколько лет у вас есть;
  4. Не будьте слишком критичны. Более вероятно, что если вы сделаете обзор в форме беседы и предложений, он будет воспринят лучше, особенно если рецензент чувствует, что он также принимал участие в принятии решений. Вместо того, чтобы говорить: «Вы должны переписать эту функцию так или иначе, потому что она совсем нехорошая», вы могли бы сказать: «Каково ваше мнение о следующем решении…? Я думаю о том, чтобы разделить эту функцию на 2, потому что……. ».
  5. Можно не соглашаться, если вы можете объяснить «почему» и у вашего несогласия есть веские причины, и вы не просто защищаетесь.
  6. Приятно чувствовать, что тебя ценят и уважают. Не стоит недооценивать небольшую похвалу или спрашивать чье-то мнение.
  7. Наконец, всегда помните, что мы делаем обзоры кода, чтобы помогать друг другу. Это не следует рассматривать как личную атаку (если только отзыв не сделан таким образом, что человек явно затаил на вас злобу и на самом деле пытается вас унизить, к счастью, я никогда с этим не сталкивался).

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

Дополнительная литература