Code review

Code review

Правила подготовки к code review

Как улучшить code review?

• Учитесь быстрее — правильно составляйте список изменений в коде, это поможет ревьюеру сконцетрироваться на главном и дать полезный фидбек.
• Делайте других лучше — подавайте пример коллегам.
• Избавьтесь от конфликтов в команде — code review это всегда споры. Относитесь к ним обдуманно и последовательно.

Золотое правило: цените чужое время.

Старайтесь сами находить ошибки, это полезно для вас обоих. Относитесь к ревьюеру не как к препятствию, а как к достойному доверия союзнику.

Техники подготовки к код ревью

• Сделайте первое ревью самостоятельно

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

• Напишите понятный список изменений

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

• Пусть ваш код говорит сам за себя

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

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

• Делайте узконаправленные изменения

Лучшая тактика — одно изменение для одной проблемы. В противном случае ревьюеру будет сложно понять, какие изменения служат цели А, а какие цели Б.

• Разделяйте длинные changelist’ы

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

• Адекватно воспринимайте критику

При подготовке к code review морально настройтесь, что скорее всего вы получите в том числе негативный отклик. Лучший способ угробить code review — принять критику слишком близко к сердцу. Это особенно легко сделать, когда ревьюер формулирует фидбек как личные претензии.

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

• Будьте терпеливы к ошибкам

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

• Задавайте вопросы правильно

Если попадается непонятное замечание, спросите: «Что будет полезно изменить?». Также можно попробовать угадать намерения ревьюера и предварительно отредактировать код — это может помочь, даже если вы не совсем попали в цель.

• В неоднозначных ситуациях отдайте преимущество ревьюеру

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

• Оперативно отвечайте на комментарии

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

Принципы хорошего код-ревью

• Уважайте друг друга.
  • Будьте доброжелательны и помните, что вы оцениваете не человека, а его код. Это может казаться мелочью, но это база, из которой складывается здоровая атмосфера в команде и эффективная работа.
• Будьте объективны.
  • Обязательно ссылайтесь на источники, документацию, материалы, которые помогут разработчику быстрее решить проблему. Если вы столкнулись с тем, что человек написал код не так, как это принято на проекте, отправьте ему ссылку на код коллеги.
• Не давайте готовое решение.
  • Задача ревьюера — подтолкнуть человека в правильную сторону, подсказать, как ещё можно подступиться к задаче, какие инструменты можно использовать.
• Учитесь в процессе код-ревью.
  • Относитесь к код-ревью как к мощному инструменту для обучения.

Аспекты которые надо проверить

Соответствие требованиям

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

Код

• Стиль кода
  • Перед ревью нужно убедиться, что в коде нет ошибок или предупреждений

ESLint.

• Нет console.log(...)
• Читаемость кода
  • Код должен читаться легко. Если в коде много сложных map/reduce, от которых можно было избавиться - надо исправлять. Если функция сложная и занимает 100 строк - надо разбивать на небольшие понятные функции. Если логика усложнена и можно было реализовать проще - надо переписывать. Смотри принцип KISS
• Работоспособность кода.
  • Код работает и выполняет свои прямые обязанности, логика корректна, и т. д.
  • У циклов есть установленная длина и корректные условия завершения.
• Понятность кода.
  • Код прост для понимания.
  • Названия методов не слишком длинные.
• Соответствие кода принятому стилю оформления.
  • Правильно названы пространства имен / классы / методы / переменные. Нет бесполезных x, a, p, которые не несут смысла. Все названия функций соответствуют тому, что они делают, а классы отражают реальные бизнес объекты.
  • Соблюдены правила именования файлов в соответствии с классами.
  • Соответствует пространство имен класса и физическое расположение файлов.
• Избыточность кода.
  • Отсутствуют повторяющиеся части которые можно вынести в отдельную функцию.
  • Отсутствуют методы, которые можно в коде заменить на библиотечные функции.
  • Отсутствует закомментированный ненужный код.
  • Отсутствуют глобальные / статические переменные от которых можно избавиться или переместить их.
• Независимость кода.
  • Код является независимым, насколько это возможно.
• Обновление конфигурации.
  • При необходимости добавлены изменения в конфигурации.
  • Есть описание изменений к релизу.
  • Доработаны инструменты установки / развертывания (например, npm-пакеты).
• Корректная обработка исключений.
  • Исключения используются по предназначению.
  • Сохраняется информация о произошедшей ошибке (например, в лог)
    • Нет пустых блоков catch.
  • Представлены внятные пояснения к произошедшей ошибке.
    • Сообщения локализованы.
• Предусмотрена безопасность.
  • Все входные данные проверяются (на корректный тип, длину, формат, диапазон).
  • Все выходные данные проверяются и при необходимости кодируются (например, от XSS).
• Повторное использование
  • Всегда нужно придерживаться принципа DRY (don't repeat yourself). Не должно быть повторяющихся кусков кода. Также функции, которые могут в дальнейшем использоваться повторно должны быть спроектированы соответствующим образом. Если такие функции уже были ранее в коде, необходимо проверить, что в MR не изобретают велосипед, а используют то, что уже наработано.

GIT

• Pipeline прошёл успешно.
  • У MR стоит зелёная галочка в кружочке.
• Название и описание MR соответствуют принятым требованиям к оформлению.
• Отсутствуют неиспользуемые npm пакеты.
• Корректные комментарии к коммиту.
  • Комментарий к коммиту отражает внесенные доработки.
  • Система контроля версий используется по прямому назначению (например, не содержит напоминаний вида “не забыть обновить …”, “TODO”)?
• Соответствие комментариев принятым требованиям оформления.
  • Комментарии соответствуют принятым требованиям к оформлению и правилам (русского) языка.
• Атомарность коммита.
  • обычно коммит не атомарен если в его описании присутствует союз “и”.

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

• Есть комментарии в коде.
  • Комментарии раскрывают смысл кода.
  • Все функции и их параметры прокомментированы с использованием JSDoc.
• Соответствие комментариев принятым требованиям оформления.
  • Комментарии соответствуют принятым требованиям к оформлению и правилам (русского) языка.