Комментарии 13
Спасибо за пост.
Code review - это процесс, в котором другие разработчики программного обеспечения проверяют код конкретного инженера с целью проверить его соответствие стандартам, выявить ошибки, выявить несоответствия в стиле кодирования и .... и в целом характеристикам качественного кода.
Вот это не задача Code review. Это задача инструментов, которые проверяют код в автоматическом режите. Например: pre-commit, pre-receive, SonarQube, tslint, prettier и так далее.
Несмотря на все вспоминаемые плюсы, у code review есть и недостатки. Первый - это длительность процесса, потому что некоторым людям приходится тратить время на просмотр кода, а другим - на его исправление. Другой недостаток - повышенная стоимость реализации проекта, так как разработчикам приходится оплачивать время.
Автоматизируйте все что можно. Я написал в этом комментарии как и какими инструментами можно автоматизировать и можно сократить расход времени и денег.
Покрытие тестами - Есть ли тесты для модифицированного или нового кода?
Для этого есть инструменты. Ищутся так: Test Gap Analysis и Minimize Your Regression, например SonarQube.
Есть ли потенциальные проблемы с безопасностью в коде? Создает ли код новые дыры в безопасности системы?
Для этого есть инструменты SAST, DAST, semgrep и другие.
Есть ли код, который случайно указывает на тестовую базу данных,
Тестировать бд нужно в Database Lab.
Я посту поставил плюс, так как code review действительно нужно.
Вот это не задача Code review.
Типичная ситуация: ищут проблемы не там, где они могут быть, а там, где их легко найти. Ну поставите вы пробел с другой стороны - что от этого изменится? Да ничего.
Автоматизируйте все что можно.
И да, и нет. Во-первых, автоматические инструменты достаточно "костные" -- вылезает стопицот случаев, где реально такое написание оправдано, однако нет возможности объяснить это аналайзеру. А во-вторых, они реагируют лишь на примитивные случаи подобно потенциальным ошибкам, оставляя за кадром более глобальные вещи как общий дизайн, стиль и читаемость кода, что например для меня является гораздо более важным аспектом, нежели наличие потенциальных ошибок, которые легко покрываются тестами.
Спасибо за комментарий и отличный список инструментов. Совершенно согласен, что автоматизировать нужно по максимуму.
Получился какой-то code review в идеальном мире. Интересно как часто на все это действительно проверяют? Это сколько же нужно тратить время на ревью, чтобы полностью вникнуть в задачу и насколько хорошо код решает эту проблему, с учётом того что зачастую есть куча легаси в который лучше не лезть и поэтому приходиться городить костыли.
Как написали выше, зачем проверять форматирование? Если это все хорошо решается автоматически
Проверка на покрытия тестами? Вы 100% кода покрываете тестами? Зачем? Довольно большая часть кода этого не требует
Спасибо за комментарий. Смотрите, такой вопрос. А откуда появился такой легаси, в который даже страшно заходить? Как он разрабатывался и принимался? Проводятся ли процедуры рефакторинга и реструктуризации кода? Был ли ранее процесс code review и как его проводили?
Возможно, раньше его не проводили. Всякое бывает. Разработка MVP, потом все начинает обрастать новым кодом и...добрый вечер... имеем не очень качественную кодовую базу. Как быть тут? Как команде вникать в задачу и понять тот код, который разработан? Тут в дело вступают процессы в команде. Как работают сеньоры, как они работают с джунами, какой процесс адаптации и обучения, какие еще практики применяются? Все это можно поэтапно вводить в команду. К примуру, практики экстремального программирования - совместное владение кодом, групповые ревью, парное программирование и т.д.
Насчет форматирования... Это то, на что надо обратить внимание. Как вы это будете делать - это ваш выбор. Форматирование из IDE с принятыми настройками или какие нибудь автоматические инструменты. Не важно. Главное делать это и отлавливать. Код будет читаемым и его легко дальше передавать.
100% покрытие тестами - это БАЦ (Большая амбициозная цель). Главное, чтобы тесты покрывали максимально возможные кейсы, которые рождаются из требований и критериев приемки. Можно запустить процесс, который в долгую приблизит ваш продукт(ы) к БАЦ.
Все видимо сильно зависит от компании. В кровавом энтерпрайз часто дикая конкуренция, нужно быть на шаг впереди, при этом ресурсы сильно ограничены. При этом программисты часто начитаются про всякие совершенные архитектуры, паттерны и что все нужно покрывать тестами и вообще код должен быть такой, что его хоть сейчас выложи на гитхаб и все скажут какой он прекрасный. И этим болеют не только джуны, но и синьоры. Нужно чётко понимать, что бизнес не продаем идеальный код, пока вы его будете вылизывать, конкуренты реализуют эту фичу работающую хоть как-то, но в два раза быстрее. Главная задача это баланс между говнищем которое не возможно поддерживать и супер идеальным кодом который уже не продашь. Мне в этом плане очень понравилась методология: Разработка через страдание. Смысл примерно следующее: сначала сделай чтобы было, потом чтобы было красиво, а потом чтобы было быстро.
Про тесты и покрытие 100% это вообще адское зло, процентов 80% кода вообще не нуждается в тестах и даже вредно и такой код становится дорого поддерживать.
Попробуйте применить ваш список правил к своему посту. Например, написать слитно "вдолгую" и "неоценим". И поставить запятую после "я не понимаю"! А после можете осознать, что все эти ошибки не влияют на суть вашего поста. И замечания вроде моих не привносят в процесс ничего кроме раздражения. Нет? Тогда давайте перепишем весь пост и разобъём его на абзацы немного иначе...
Важно не только на что за замечания вы делаете, но и как и зачем вы их высказываете. И если не понимать этого, то можно потерять время и даже сотрудников.
Я вам скажу за ваше ревью спасибо. Да, опечатка не влияет на смысл поста. Но, согласитесь, куда приятнее читать пост без ошибок.Тут очень важна культура в команде, компании и под каким соусом подается код ревью. Для разработчика сode review - это не инструмент порицания, а инструмент оттачивания своего мастерства (обучения).
Важный нюанс: никогда в команде не взлетит культура ревью, если нет культуры декомпозиции задач. Если взято за правило бахать коммитами по 1000 строк в 50 файлах, то либо ревью по чек-листу будет занимать дни, либо все скатится в "ок, это код, ревью пройдено".
Code Review. 80 lvl