22 May

Code review Терминатор. Ревью, за которое вам скажут спасибо

Website developmentProgrammingPerfect codeGitHub

Рыжик помогает мне ревьюить код. А когда ему что-то не нравится — тоже настоящий Терминатор

«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. Эта статья — вольный перевод моего же оригинала на английском. Сконвертировал из «перевода» в «статью», чтобы не сбивать с толку читателей.
Tags:code reviewревью кодаревьюgithub
Hubs: Website development Programming Perfect code GitHub
+26
5.3k 74
Comments 15
Top of the last 24 hours