За пять лет работы в Яндексе я участвовал в разработке одиннадцати проектов. Писал код на JavaScript, Python и C++. Некоторые проекты делал в одиночку, другие разрабатывал в группе из восьми человек. Но в каждой команде, на всех проектах, вне зависимости от языка программирования я использовал кодревью.
Я люблю ревью, с его помощью постоянно узнаю что-то новое. Иногда, глядя на чужой код, хочется воскликнуть: "А что, так тоже можно?" В чужом коде я нахожу интересные приёмы и беру их себе на вооружение. Много новых знаний черпаю из комментариев к моему коду. Для меня стало открытием, что люди любят делиться своим опытом. Даже когда я разрабатываю проект в одиночку, то прошу ребят из другой команды посмотреть пои пулреквесты. Это мотивирует меня писать красивый понятный код.
Но я не всегда был сторонником этой практики. Вначале, ревью было для меня наказанием. Я мог неделю с вдохновением писать код, вкладывая в него все силы. Отправлял пулреквест, трижды пинговал ревьювера, а в ответ получал сухое "вроде ок" или, что ещё хуже, десятки злобных комментариев не по существу.
Мне на ревью приходили пулы из пяти тысяч строк. Я часами пытался разобраться в коде, по сотне раз скроллил от функции к тесту и обратно. Писал десятки бесполезных комментариев о пропущенной точке с запятой. Всё это жутко меня раздражало. Часто откладывал ревью на потом, и у меня накапливались десятки непросмотренных пулов.
Если вы чувствовали это на себе, значит, статья для вас. Я расскажу о приёмах и инструментах, которые использую каждый день на протяжении пяти лет ежедневного кодревью.
Представьте, что решение задачи - это приготовление блюда. Вы работаете в команде, поэтому вам нужно не просто приготовить, а ещё и научить этому других поваров. Не достаточно показать им результ, нужно записать рецепт.
Каждый шаг рецепта - это коммит: разбили два яйца - закоммитили, добавлили стакан молока - закоммитили, насыпали двести граммов муки - снова закоммитили.
В каждом коммите я выражаю одну простую мысль. Это может быть реализация метода модели или компонент в вёрстке. Так ревьюверу будет проще меня понять. Я не сваливаю на него всю задачу, которую невозможно проглотить за раз, а рассказываю о решении по кусочкам.
Любой рефакторинг выношу в отдельный коммит. Зачастую рефокторинг носит чисто технический характер, например, переименование метода. Ревьюверу не нужно вчитываться в каждую строчку такого изменения. Он пробежит глазами "по диагонали" и сможет уделить больше времени по-настоящему важному коду.
Крошите, разбивайте, измельчайте свой код на маленькие коммиты. Это позволит ревьюверу лучше разобраться в вашем коде. Ничего страшного, если вы переборщите с декомпозицией. Два коммита легко объединить в один. Гораздо сложнее разделить большой коммит на несколько маленьких. "Нарезанные овощи" легко получить перемешивая "нарезанные помидоры" и "измельчённый лук". Но чтобы получить из тарелки салата все ингредиенты по частям, нужно потратить намного больше времени [Q1, Q2, Q3].
После коммита я сразу пушу изменения на гитхаб. Это пару раз выручало меня, когда случались "кофейные неприятности" с ноутбуком.
Когда пишу емэйл, то заполняю заголовок и содержимое письма. Заголовок — короткое и ёмкое название, тело письма — развёрнутое, подробное описание с картинками и ссылками. Такой же подход применяю к описанию коммитов.
Я давно отказался от коммитов командой git commit -m 'fix1'. Вместо этого выполняю команду git commit, которая открывает текстовый редактор. В первой строке указываю короткое и ёмкое описание коммита (как в заголовке письма). После пробельной строки описываю детали реализации (подобно содержимому емэйла).
Не люблю рецепты, где пишут "добавьте щепотку соли" или "выпекайте до готовности". Щепотка у каждого своя, а глядя на курочку в фольге я не могу определить её готовность. В описании коммитов стараюсь избавляться от всех неоднозначностей. При помощи ASCII-графики описываю тестовый пример. Если решению предшествовало обсуждение, где мы рисовали схемы на доске или в блокноте, то прикладываю к описанию ссылку на фотографию [Q5, Q6].
(пример описания коммита с использованием ASCII-графики)
(пример описания пулреквеста с заголовком и телом. Для редактирования комментария я использовал консольный редактор vim)
(пример отображения коммита с описанием на GitHub. По стрелкам в правом верхнем углу можно перемещаться между коммитами)
Перед подачей блюда повар снимает пробу, выкладывает на красивую тарелку, добавляет украшение. Мы будем делать это тремя командами:
git status
git diff comments.js
git add comments.jsЯ выполняю их столько раз, сколько файлов изменил. Намеренно не делаю git add ., чтобы посмотреть каждый файл по отдельности. Так я проверяю самого себя, смотрю на свой код свежим взглядом.
Не представляю свою жизнь без линтера. Это инструмент, который автоматически проверяет соответствие кода выбранному стилю. Для JavaScript я использую ESlint. Можно сравнить линтер с роботом R2-D2 из "Звёздных воин", который ходит по коду и приводит его в порядок. Места, которые он не может исправить сам, подчеркивает красным.
С моим любимым WebStorm этот линтер работает "из коробки". Я вижу и исправляю проблему сразу, как только написал код, а не оставляю эту работу на потом. Перед коммитом линтер запускается автоматически при помощи husky.
Код после линтера приятно смотреть. Я избавляю ревьювера от необходимости писать десятки бесполезных комментариев о пропущенном пробеле. Всё внимание будет сконцентрировано на действительно важных вещах.
(пример запуска линтера на коммит)
Если коммит - это шаг рецепта, то пулреквест - это рецепт целиком. Когда все шаги описаны хорошо, то написать рецепт не составит труда. Можно создать описание пулреквеста командой git log --pretty='%h: %B' --first-parent --no-merges --reverse.
(пример выполнения команды `git log --pretty='%h: %B' --first-parent --no-merges --reverse`)
(пример описания пулреквеста текстом, если скопировать результат вывода предыдущей команды)
Хеши коммитов становятся ссылками, по которым может навигироваться ревьювер. Они остаются рабочими, даже если объединить все коммиты в один. Подробнее про объединения поговорим дальше.
Пулреквест готов! Но не спешите закрывать задачу, самое интересное ещё впереди.
Давайте ненадолго отвлечемся от создания пулреквеста и представим себя на месте ревьювера - человека, которому прислали код на проверку.
Перед тем как смотреть код, я стараюсь понять какую задачу решает автор. Читаю описание пулреквеста, перехожу по ссылкам в ишью, изучаю схемы и фотографии. Иногда, ошибка кроется в постановке задачи. Хороший ревьювер оценивает идею, а не выступает в роли "продвинутого линтера".
В любой непонятной ситуации задаю вопрос. Даже если он кажется мне глупым или банальным. Таким вопросом я могу натолкнуть автора на правильную мысль.
В некоторых случаях я не согласен с предложенным решением. Это нормально! У каждого программиста своё мнение. Если код автора объективно хороший, то я не пытаюсь переубедить его. Максимум высказываю предложение, подкрепляя его ссылками, бенчмарками или картинками [Q6]. Это позволяет вести конструктивный диалог, а не переходить на личности.
Код автора может выглядеть не оптимальным на первый взгляд. Он написан так, как принято делать в проекте. В таком случае лучше оставить вариант автора.
Разберёмся на примере. Автор написал функцию суммирования массива чисел таким образом:
function sum(arr) {
return arr.reduce(function (res, i) {
return res + i;
}, 0);
}
sum([1, 2, 3]); // 6Такой код можно переписать компактнее, используя стрелочные функции:
const sum = arr => arr.reduce((res, i) => res + i);Но проект начинали тогда, когда ещё не было стрелочных функций. Если написать компактное решение, то код перестанет быть единообразным, в нём станет сложно разбираться. Либо оставляем вариант автора, либо переписываем все функции на стрелочные. Главное - сохранить единый стиль.
Иногда, мне на ревью приходит очень большой и сложный пулреквест. Автор старался две недели, а мне предстоит осознать полёт его мысли и вникнуть в каждую строчку за несколько часов. Одному мне не справиться.
В таком случае я прошу автора об offline-ревью. Ставлю рядом складной стульчик (кстати, не используйте мягкий: рискуете тем, что ревью затянется надолго), сажу автора возле себя и прошу рассказать его о решении.
Эффективность такого подхода невысокая. Во-первых, тратится время двух разработчиков. Во-вторых, легко начать злоупотреблять этой практикой: слишком велик соблазн не думать самостоятельно над кодом автора (пусть придёт и расскажет). В-третьих, я погружаюсь в ход мыслей автора, невольно принимая его за правильный — так я не вырабатываю свою точку зрения.
Опасная практика, но порой без неё не обойтись. Применяйте offline-ревью с осторожностью.
Вернёмся к пулреквесту, который я готовил в первой части статьи. Ревьювер написал комментарии. Наступает этап правок и обсуждений.
Если я согласен с замечанием, то исправляю его и пишу в комментарии что исправил. Ревьюверу будет приятно увидеть, что я прислушался к нему, а я сразу вижу какие замечания поправил, а какие нет. Если не согласен, то задаю ревьюверу вопросы: почему он так считает? правильно ли он понял мою мысль? чем он может подкрепить свою точку зрения? Стараюсь завязать конструктивный диалог, в ходе которого мы докопаемся до истины.
Я отвечаю на комментарии во вкладке с файлами. Так ревьювер получит одно письмо со всеми отметками и вопросами, а не по письму на каждый комментарий.
(пример ответа на ревью: комментируйте на вкладке с файлами и благодарите ревьювера в конце)
В конце обязательно благодарю ревьювера. Он потратил своё время на то, чтобы вникнуть в мой код и сделать его лучше. Разве это не заслуживает приятных слов? В следующий раз этот человек возьмётся за мой пулреквест с бо'льшим энтузиазмом, потому что видит важность своих советов и уважение с моей стороны.
Разделение на микрокоммиты помогает на кодревью. Для истории такое разделение избыточно. До этого я старался разделять код на коммиты так, чтобы каждый коммит описывал шаг рецепта. Настало время объединить коммиты пулреквеста в один, чтобы получить готовое блюдо, которым будет легко оперировать при сервировке стола.
Объединить коммиты можно командой git rebase --interactive master. Напомню, что я работаю в фичебранче FEATURE-1, а master - это название основной ветки. После выполнения команды откроется текстовый редактор, в котором первый коммит оставляю без изменений, а у второго и последующих заменяю pick на squash. Так содержимое второго и последующих коммитов добавляется в первый.
(пример ребейза, в котором четыре последние коммита сливаются в один)
После этого нужно запушить изменение с флагом --force. Я преписал историю, и на удалённом сервере возник конфликт локальной версии и ранее известной. Командой git push origin FEATURE-1 --force я сообщаю серверу, что локальную версию изменений нужно считать правильной. Все готово для того, чтобы влить изменения.
Можно обойти трудности описанные выше используя новые возможности интерфейса GitHub. Для этого перед мержем выбираем пункт "Squash and merge".
(пример объединения коммитов при мерже через интерфейс GitHub)
После приготовления блюда повар убирает своё рабочее место, моет ножи и вытирает стол. Я сделаю то же самое с веткой FEATURE-1. Удаляю локальную версию командами:
git checkout master
git pull origin master
git branch -D FEATURE-1
(Серверную версию ветки можно удалить по кнопке сразу после мержа пулреквеста.)
В заключение, ещё раз коротко о командах, которые я использую:
# Создаю и переключаюсь на фичебранч
git checkout -b FEATURE-1
# Отсматриваю изменения
git status
git diff src/controllers/v1/comments.js
git add src/controllers/v1/comments.js
# Создаю и пушу коммит
git commit
git push origin FEATURE-1
# Готовлю описание к пулреквесту
git log --pretty='%h: %B' --first-parent --no-merges --reverse
# Исправляю историю после ревью
git rebase --interactive master
git push origin FEATURE-1 --force
# Удаляю фичебранч
git checkout master
git pull origin master
git branch -D FEATURE-1Спасибо Вадиму Макишвили за редактуру и полезные вопросы. Олегу Мохову за рекомендации, после которых я чуть больше чем полностью переписал статью. Тане Занаховой, за вычитку всех орфографических и пунктационных ошибок. Без вас ничего бы не получилось. Спасибо вам большое!
# Добавляем файлы, которые нужно прикрепить к последнему коммиту
git add comment.js
# Прикрепляем к последнему коммиту
git commit --ammendЕсли правки нужно внести в более ранний коммит, то есть два варианта. Самый простой - сделать новый коммит и объединить его со старым. Вариант посложнее — выполняя команду git rebase --interactive master, нужно перенести строчку с новым коммитом под старый и заменить pick на squash.
Если в рабочей директории нет изменений, то вы можете сразу выполнить git rebase --interactive master и заменить pick на edit возле того коммита, в который нужно внести исправления.
Нет проблем, если разные файлы попадают в разные коммиты. Чуть сложнее становится, когда один файл нужно разбить на несколько коммитов. Для этого подходит частичное добавление. Сделать это можно командой git add --patch test/comment-test.js
При выполнении команды git rebase --interactive master первым делом нужно определить порядок коммитов. Выстраивайте строки с коммитами в нужном вам порядке. Строки с ненужными коммитами удаляем. Если нужно объединить несколько коммитов, то у первого оставляем отметку pick, остальным заменяем pick на squash.
Копируем картинку в буфер обмена и вставляем в любое поле ввода на гитхабе. В комментарии к issue, коммиту или в поле описания пулреквеста. Спустя несколько секунд получим ссылку на картинку. Я установил на сотовый телефон программу Office Lens. Она вычищает лишний мусор с фотографии, кропает, убирает искривления и позволяет быстро пошарить картинку в любой чатик.
Если по делу и без злоупотребления. В общем, всё как в жизни.
