Рыжик помогает мне ревьюить код. А когда ему что-то не нравится — тоже настоящий Терминатор
«Code review Терминатор», — однажды назвал меня коллега после особо продуктивного ревью. С одной стороны, это тешило ЧСВ и было приятно. С другой — коллега действительно научился чему-то новому, и это позволило писать ему более качественный код. Так что win-win.
После смены работы сменились и коллеги. Но и на новом месте тоже начали благодарить за ревью. Решил разобраться, почему, и разложил по полочкам. Получилось 11 рекомендаций.
1. Относитесь к ревью, как к одной из главных активностей
Оставить парочку комментариев вида «А тут не хватает проверки на null» недостаточно. Надо:
- Разобраться, что нужно было сделать
- Понять, как это было сделано
- Поразмыслить о других возможных подходах. Есть ли лучше (более читаемые, поддерживаемые, быстрые)?
- Если текущий подход оптимальный, убедиться, что он реализован корректно. Иначе — предложить другой и объяснить, почему он может быть лучше.
2. Компенсируйте отсутствие невербальных сигналов
Одно дело — сидеть с коллегой рядом и что-то обсуждать, смотря в один экран. Другое дело — ревьюить код на гитхабе или битбакете. Легко неправильно понять намерения коллеги, когда общение происходит текстом. Возьмём фразу «В этой строке баг». Да, понятно, что фраза сообщает, что в статье баг. Но она могла быть сказана с разными эмоциями: злостью, удивлением, радостью о том, что баг отловлен и не попадёт на продакшн. А может, и с безразличием.
Ваш коллега может неправильно понять ваши намерения — а это ведёт к обидам, напряжению в команде и в целом фигово.
Сделайте себе жизнь проще: смягчите фразу, преобразуйте её в вопрос или, быть может, смайлик добавьте.
3. Выделите время
Чтобы убедиться, что код работает правильно и хорошо, вам нужно полностью его понимать. А на это нужно время; убедитесь, что выделяете его достаточно. Помните, что на ревью частей приложения, которые вы не знаете, понадобится ещё больше времени.
Вообще, это обратная сторона хороших ревью: это времязатратно. Если у вас нет времени, чтобы сделать его хорошо, попробуйте отложить или назначить кого-то другого (звучит как совет прокрастинатора, но ситуации бывают разные). Если это не вариант, а сделать ревью надо — помните, что вы жертвуете качеством. Хоть и необходимый, но компромисс. Если превратите его в привычку, то получите больше технического долга в перспективе.
4. Не делайте допущений; спрашивайте
Когда натыкаетесь на странность в коде или чересчур сложное решение казалось бы простой задачи — не считайте по умолчанию, что коллега сделал ошибку или плохой выбор. Возможно, он в курсе каких-то обстоятельств, о которых вы не знаете. Вы можете просто уточнить и избежать этим риска неудобных и напряжённых ситуаций, если вы кого-то несправедливо обвините. Например: «Может, можно использовать здесь такой-то класс? Насколько я понимаю, он бы тут хорошо подошёл и это было бы проще предложенной реализации».
5. Отлавливайте ситуации, когда нужно обратиться напрямую
Обычно ревью проводят асинхронно. Так можно и в код погрузиться, и коллегу меньше отвлекать. Но в некоторых ситуациях лучше напрямую обратиться (подойдя или позвонив):
- Когда сроки поджимают. Быстрый фидбек ускоряет решения. Но аккуратно: при дедлайнах и так все на взводе, и отвлечения будут раздражать и сбивать концентрацию.
- Грубые ошибки. Публичное их обсуждение может кого-то очень сильно смутить и расстроить. Лучше лично поговорить и разъяснить проблему. Может быть, это просто недосмотр, а может, пробел в знаниях — который теперь заполнен.
- Выбран полностью неправильный подход. Говорить человеку о том, что нужно выбросить его работу, нужно аккуратно. Лучше использовать язык тела и интонацию, чтобы донести уточнения без обид.
Ну и хорошей идеей будет добавить потом в ревью-систему сводку разговора.
6. Прочитайте сначала тикет
Часть требований может быть не покрыта во вроде бы правильном пулл реквесте. Чтобы избежать такого — просто прочитайте сначала постановку задачи. Заодно это поможет в понимании того, что в целом в изменениях происходит.
7. Обоснуйте ваши предложения
Некоторые ошибки (опечатки, например), не нуждаются в объяснениях. Но если вы предлагаете альтернативное архитектурное решение или другое наименование — поясните преимущества и недостатки обоих вариантов. То, что кажется очевидным вам, может быть совершенно не очевидным другим людям.
Кроме того, есть пословица: «Дайте человеку рыбу, и вы накормите его на день. Научите его рыбачить, и вы накормите его на всю жизнь». Когда вы научили коллегу новому подходу, он сможет использовать его в будущем и писать код лучше. Заодно ещё и бонусом улучшится качество проекта.
8. Хвалите хорошие решения
Ревью не обязано быть перечислением ошибок. Если вы увидели хорошее место, интересное решение, полезный метод — скажите про это. Краткий комментарий «Классное решение» может улучшить человеку настроение на весь день. Да, и не хвалите плохие пулл реквесты: это натянуто и рушит смысл идеи.
9. Будьте вежливыми
Подсказка: фраза вида "Can we please get rid of the brain-damaged stupid networking comment syntax style?" не является вежливой (простите за английский тут, но это прямая речь Линуса Торвальдса и её было бы стрёмно переводить; он там красочно настаивает не использовать определённый вид комментариев, для вида вежливости добавляя «please»).
10. Помогайте
Если в процессе ревью вы проанализировали несколько файлов и в результате нашли альтернативное решение — скиньте коллеге ссылки на эти файлы. Можно даже с номерами строк, так ещё удобнее. Это не займёт много ваших усилий, а вот коллеге может сэкономить порядком времени.
11. Предлагайте, не указывайте
Когда вы на ревью предлагаете другой подход — не говорите в приказном порядке коллеге использовать ваш вариант. Аргументируйте и позвольте коллеге решить. Скорее всего, он примет ваш совет. А если нет, в чём может быть причина?
- Оба подхода примерно одинаковы. Если нет объективных причин для выбора нового подхода, то нет причин тратить время и применять его. Disclaimer: единообразие кода — объективная причина.
- Есть объективная причина, по которой ваш подход лучше. Но почему-то автор кода её не понимает. Пересмотрите вашу аргументацию, объясните ещё раз — а заодно проверьте, не ошибаетесь ли вы сами.
- Личные конфликты. Это скользкий лёд и действовать тут нужно аккуратно. И это уже выходит за рамки темы ревью.
На этом всё. Суммируя:
Делайте мир вокруг вас чуточку лучше. Делайте хорошие ревью.
UPD. Эта статья — вольный перевод моего же оригинала на английском. Сконвертировал из «перевода» в «статью», чтобы не сбивать с толку читателей.
