Проверка кода может быть сложной задачей, но она не обязательна.

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

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

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

1. Различайте «плохо» и «не так, как я бы это сделал».

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

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

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

2. Убедитесь, что автор чему-то научился из ваших комментариев.

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

Проверка кода - это благодатная почва для передачи знаний о продвинутых функциях языка / фреймворка, передовых методах, предметной области и знаниях компании. Научите менее опытных разработчиков кешированию, производительности, стабильности, масштабируемости и уязвимостям. Если бы мне пришлось это количественно оценить, я бы сказал, что около 30% того, что я узнал за последние несколько лет, было получено из обзоров кода (великолепный метод Ruby tap - лишь один из примеров).

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

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

3. Автор несет ответственность, и поэтому последнее слово остается за ним.

Когда спор не может быть урегулирован и дальнейшее обсуждение его бесполезно, последнее слово остается за автором, который (обычно) имеет лучший контекст для предлагаемых изменений и несет за это ответственность. Да, вы меня слышали. Не более опытный разработчик или технический руководитель. Автор.

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

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

4. Приучите себя к маленьким PR

Маленькие пиарщики похожи на пиццу: они делают всех счастливее. У рецензентов меньше работы, и они могут легко рассуждать о коде, предлагаемом изменении, его качестве и, следовательно, могут дать лучшую обратную связь. С другой стороны, автор получает обратную связь быстрее - и она обычно лучше. Более того, с цепочкой небольших PR, которые объединены в большую функцию, фрагменты кода проверяются раньше, что делает отслеживание PR все лучше и лучше.

Мое эмпирическое правило состоит в том, что PR должен быть меньше 12 файлов. Это моя золотая середина.

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

По моему опыту, есть несколько способов сделать это эффективно (в порядке предпочтения):

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

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

3. Если ваша ветка уже содержит большой набор изменений, вы можете разбить его на более мелкие «проверяемые» фрагменты и, при желании, создать запросы на вытягивание для каждого из них. Есть несколько способов сделать это (например, разделить на значимые коммиты с помощью git interactive rebase для создания PR с подмножеством изменений).

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

5. Комплименты

Меня вдохновило приложение для фитнеса, которое я использую, которое хвалит меня, когда у меня все хорошо:

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

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

Это моя гифка, чтобы дать понять товарищу-разработчику, что они хорошо поработали:

Я призываю вас найти свою красивую нишу.

Это очевидно, но также стоит упомянуть: будьте вежливы! Помните, что суть в том, что вы критикуете чью-то работу!

Поэтому замените «делать» и «должен» на «учитывать» и «как вы относитесь к…?» Включение «пожалуйста» и смайликов здесь и там тоже не повредит.

Заключительные мысли

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

Спасибо за прочтение!