Code review неизбежен. Поэтому к нему необходимо готовиться… слегка заранее. Возможно, даже за пару лет. В общем, эта статья будет полезна для как для тех, кто уже проверяет чужую работу, так и для молодых специалистов, у которых Code review впереди. В прошлом году Google поделились собственным опытом, который и будет кратко и адаптировано изложен в этой статье.
Вспомните о вашей истинной цели
Code review проводится для того, чтобы проверить код, исправить ошибки в нём и улучшить его, а не для того, чтобы уязвить своего коллегу. К том уже, если вам предстоит оценивать код, который является частью крупного и многоуровневого проекта, делайте это с позиции всей системы, а не той части, которую вам предоставили.
Не забывайте: вы лишь оценщик
А не великий гуру или вершитель судеб. Поэтому здраво оценивайте и код, и свою критику. Старайтесь быть адекватным, корректным и вежливым в своих формулировках, иначе есть шанс того, что разработчик разочаруется не только в себе и своих навыках, но и в профессии.
С другой стороны, не позволяйте накапливаться проблемам. Костыль способен не выделяться и в общем-то совершенно не портить продукт, если он там один. В лучшем случае — полтора. Но когда приложение переполнено неполадками… Не надо так.
Обратная связь
Обязательно оставляйте обратную связь после проверки кода. Лучше всего не только вежливо указать на ошибки, но и объяснить, как их исправить, и рассказать, почему они появились вообще. Это необходимо, чтобы в следующий раз разработчик смог сделать код лучше и, при том, самостоятельно.
Google так же рекомендует использовать в случае написания рекомендаций префикс «Nit:» (от слова «nitpicking», придирка). Это не обязательно, просто «Nit:» является универсальной пометкой, которую в большинстве случаев вам не придётся пояснять другим людям (а ещё по ней удобнее искать в логе).
Вот несколько рекомендаций о том, как писать комментарии после ревью:
- представьте водопад. Отлично. Теперь начните писать, не забывая, что вы с создателем кода не враги, а коллеги. И вы решаете одну задачу: будьте благожелательны и терпимы. В конце концов, все ошибаются;
- если даёте какие-то указания, поясняйте их. Это выглядит менее жёстко и грубо, а также поможет программисту понять, что от него вообще хотят;
- объясняйте, однако не делайте всю работу за кодера. Это его код, как минимум потому, он всё исправит лучше и аккуратнее. Тем более, если это штатный и постоянный сотрудник — ему нужно понять ошибку, узнать, как писать код, чтобы такого не было. Поэтому лучше будет, чтобы он всё исправлял своими руками (за исключением совсем печальных случаев, конечно, — если со специалистом всё плохо, меняйте код сами);
- если код очень громоздкий и упростить его нельзя, предложите разработчику добавить к нему развёрнутые комментарии. Объясните их необходимость, чтобы человек не воспринимал просьбу как желание максимально его загрузить и поиздеваться;
- держитесь стандартов вашей команды и компании. Если кодер жалуется на придирчивость, говорите ему, что он сам должен был знать, на что шёл. Главное, чтобы никто из вас не перегибал в этом палку;
- если программист предлагает вам принять работу с условием, что он доделает её позже, неофициально, — не соглашайтесь. The cake is a lie. По крайней мере, в 9 из 10 случаев.
Обидеть художника может каждый
Если вы всё-таки вышли с сотрудником на конфликт, стремитесь к поиску компромисса всеми возможными силами. Для этого отложите эмоции и предложите поступить так же вашему разработчику, обратитесь к стандартам проекта. Поговорите лично или по видеосвязи, обычно так людям проще понять друг друга.
В случае, когда не помогло ничего из предложенного, привлеките к дискуссии опытных членов команды, которые не занимаются этой задачей. Однако не увлекайтесь, ведь разработчик не должен ждать ваш или их ответ слишком долго. Это не только губительно сказывается на работе, но и значительно снижает уровень мотивации программиста, его лояльность и вовлечённость в данный проект.
Так а что проверять-то?
Всё.
Каждую строчку.
Весь код: от и до.
А теперь списком:
- Общая структура и то, вписывается ли код в текущий проект.
- Функциональность, а именно закрываются ли с помощью этого кода поставленные задачи.
- Эргономичность: UI и соответствие общему стилю.
- Многопоточность — код не должен конфликтовать с другими элементами кодовой базы при многопоточном выполнении. Код вообще не должен конфликтовать: ни снаружи, ни внутри.
- Лаконичность, в том числе — в прямом смысле. Код не должен быть ультрасложным и большим. Если что-то можно упростить — делайте это, если подобные действия не вредят его функциональности и качеству в целом.
- Потенциал в вопросе масштабирования, если вы заметили что-то в коде, что в будущем можно будет использовать для оптимизации или развития продукта, скажите об этом вашему разработчику и похвалите его.
- Все имеющиеся тесты, включая интеграционные, модульные и т. д., так же следует проанализировать как минимум на наличие ошибок.
- Соответствие кода стандартам: стиль и архитектура. Более подробно описано в следующем разделе статьи.
- Понятность кода. Проверьте, перепроверьте и переперепроверьте, чтобы все поля, функции и переменные (а также всё остальное) были понятны, названы однозначными именами и не нуждались в дополнительных пояснениях для гипотетической замены работающего над кодом специалиста.
О восьмом пункте
Помимо ошибок, которые заставляют итоговую разработку функционировать некорректно, внимание стоит обратить так же на стиль и архитектуру кода. Первый должен соответствовать тому, который принят в команде или компании. Если этот аспект не оговаривался с разработчиком, спрашивать с него не нужно. Он не виноват, что не читает чужие мысли.
Что касается архитектуры, она должна гармонировать с принципами, на которых основана разработка в целом. Если вы замечаете ошибку и видите решение, которое будет соответствовать им максимально, можете рекомендовать его.
Чтобы предотвратить возможные трудности в обсуждениях данных аспектов, покажите разработчику кодовую базу. Это нужно делать перед тем, как человек начнёт работать. И только в том случае, если знакомство с накопленными материалами не скажется негативно на результате (копирование, попытка имитировать чужой и непривычный стиль и т. д.).
Это понятно. А делать-то что?
Не обязательная, однако желательная последовательность действий такова:
- Прежде чем приступать к оценке кода вспомните, что представляет из себя разработка и что пытается сделать кодер. Есть ли смысл в его работе, возможно, он пытается сделать что-то не то? В таком случае его будет необходимо скоординировать.
- Изучите основную часть проверяемого кода. Если вам не совсем ясно, что из него — главное, спросите у программиста. Проверьте выделенное на ошибки и недостатки. Если найдёте — сразу же говорите о них разработчику. Что касается деталей: переходить к их оценке нет смысла, пока не исправлены основные проблемы. Как минимум потому, что в процессе устранения главных ошибок эти же детали могут претерпеть сильнейшие изменения.
- Изучите оставшуюся часть кода. Помните о логике и линейной последовательности действий. Проверяйте всё. Вообще. Можете прибегнуть к созданию модульных тестов, чтобы в дальнейшем иметь представления о том, какие изменения планирует программист.
Рекомендация от Google
Не затягивайте с Code Review. Реагируйте на готовый код и его корректировки незамедлительно, предлагайте небольшие, однако влияющие на качество кода изменения и контролируйте корректировки кодера.
При этом старайтесь уложиться в одни сутки (с учётом часового пояса разработчика). Однако если вы сконцентрированы на другой задаче в момент получения кода — сначала закройте её и лишь потом переходите к проверке. В обратном случае высока вероятность сделать некачественно сразу два дела.
Стремитесь работать быстрее, но никогда не совершайте действий в ущерб идее и качеству.
Полезные ссылки:
Статья от Google: https://google.github.io/eng-practices/review/reviewer/
Статья «Золотой бэк-енд: как сэкономить и не убить качество приложения»: https://infoshell.ru/blog/zolotoj-back-end-kak-sekonomit-i-ne-ubit-kachestvo-prilozhenija/
Статья «Удалённые разработчики: учимся взаимодействовать. 15 рекомендаций бизнесу»: https://infoshell.ru/blog/udaljonnye-razrabotchiki-uchimsya-vzaimodejstvovat-15-rekomendatsij-biznesu/
