Давно еще хотел об этом написать, но всё что-то руки не доходили. Но вот снова тема всплыла и, думаю, дай-ка напишу всё-таки. Так вот, короче, мало кто недооценивает способность людей хорошо проводить код-ревью, хотя это немаловажно, и я бы даже сказал, что очень важно. Программист точно будет участвовать в код-ревью, и если он не шарит, как в нем правильно участвовать, то это может сказаться на команде и даже процессах. Даже не звучит плохой идеей, чтобы на собеседованиях давать патч и говорить, мол, вот, отревьюй. Поэтому сегодня у нас набор ред-флагов ревьюера, которого никогда не рады видеть в своих PR.

Как в процессе ревью, что говорится, навалить кучу

✅ Начать цепляться к коду, который не трогали в рамках PR. Это классика: присылаешь PR, а к тебе залетают челы и комментят незатронутую часть файла, мол, а давай-ка тут вот всё-таки вынесем, отрефакторим, разобьем, ну и вот это всё. С такими комментариями в рамках одной задачи можно и весь проект переписать.

✅ Комментировать форматирование. Вроде в целом и правильно, с одной стороны, но с другой стороны это значит, что в проекте нет (или неправильно настроен) форматтер. И это всё тянется из PR’а в PR, и в каждый PR вы заходите и ищете там несоответствие стилю. В таком случае надо просто один раз настроить форматтер и всё. Поставьте задачу, по процессу пусть ей кто-то когда-то займется, и ваши страдания и страдания авторов PR закончатся навсегда.

✅ Предлагать переписать рабочий код другими словами без явного аффекта на результат. Условно, например, вот вы реализовали цикл while, а челик приходит и говорит: А ВОТ for РАБОТАЕТ на 1 наносекунду БЫСТРЕЕ, поэтому надо переписать и заюзать его. Причем еще ладно, если просто в проекте так принято и нигде больше нет while, но обычно это не так и везде вперемешку то while, то for, и всегда всем было насрать, но не в этот раз.

✅ Предлагать отрефакторить код, который вы просто откопипастили и всё сделали по подобию того, как уже делается в проекте. Особенно часто такое можно заметить, когда приходится заниматься каким-то легаси: постоянно приходит челик, которому надо вот именно сейчас внезапно сделать именно этот код хорошо, хоть и всё остальное плавает в говне. В таком случае, если прям надо вот отрефакторить, заводите отдельную задачу и пусть по процессу ей занимаются.

✅ Предлагать все переиграть на финальном этапе. Это тоже частое явление, когда человек не участвовал в формировании ТЗ (или даже сам намеренно скипал), а потом, когда уже весь код написан, со всеми интегрирован, возможно даже протестирован, челик приходит в PR и, оказывается, по его мнению тут все говно и надо сделать иначе. Даже если это действительно так, то уже поздно: заводите новую задачу и пусть она идет по процессу.

Вы это читаете, и, наверное, думаете, что это слишком очевидно, но в реальности это происходит просто постоянно. Делать прям хорошее ревью надо уметь, а это очень мало кто умеет. Я понимаю, конечно, что это всё чаще с благими намерениями, но в конечном итоге это редко играет в плюс.

Как стать хорошим ревьюером? Ну, для начала хватит того, чтобы хотя бы не быть плохим, а после этого можно почитать гайдики по ревью от гугла и всяких там, которых в интернете очень-очень много (просто тут в пост не помещается еще критерии хорошего ревьюера писать).

А как часто сталкиваетесь с душным ревью вы? Какие еще можете назвать его критерии? 🤔