Pull to refresh

Вещи, которые должен делать каждый: Code Review

Reading time 4 min
Views 55K
Original author: MarkCC
Code review

Самая значимая вещь, которая делает код в компании Google таким хорошим проста — code review (далее CR). Google не единственная компания, использующая CR. Всем известно, что это хорошая идея и множество разработчиков делают это. Но я не видел ни одной другой большой компании, в которой CR был бы так грамотно внедрен. В Google ни одна линия кода не уходит в production пока не получит позитивную оценку на CR.

Вы должны уделять внимание CR. Это непременное условие для разработки любого программного обеспечения на серьезном уровне. CR должен применяться не только для кода который уходит в production, но вообще для всего. Это не такая большая разница во времени как вы думаете, но разница в качестве получается огромна.

Что же вы получите от использования CR?

Очевидное: одна голова хорошо, а две лучше. Как минимум еще один человек просмотрит ваш код перед тем, как он будет проверен в реальных условиях (баги). Это наиболее часто упоминающийся профит от CR. Но мой опыт говорит, что это как раз менее всего ценно. Коллеги действительно находят ошибки в вашем коде, но подавляющее большинство таких ошибок найденных в процессе code review, прямо говоря, тривиальные вещи, которые автор кода найдет за несколько минут. Баги, на которые действительно нужно потратить время не ловятся на CR.

Самое большая польза от code review носит социальный характер. Если вы программист и вы знаете, что ваши коллеги обязательно посмотрят ваш код, вы размышляете по-другому. Вы будете писать аккуратный, хорошо документированный и организованный код, потому как вы знаете, что люди, мнение о коде которых вам важно, внимательно проверят отправленный на review код. Без CR, вы также знаете, что другие люди посмотрят ваш код. Но вы понимаете, что это произойдет не сразу, поэтому этот факт не оказывает такого подсознательного эффекта.

Другое большое преимущество — это распространение знаний. Во многих командах, у каждого отдельного программиста есть кусок кода в проекте, за который он ответственен, и этот программист сфокусирован именно на этот код. Пока коллеги не поломают что-то связанное с этим кодом, они на него не обращают внимания. Побочный эффект от этого в том, что для каждого компонента существует только один человек, который полностью понимает как он работает. Если этот человек не укладывается в сроки или — упаси Боже — покидает компанию, не останется никого, кто знаком с кодом компонента. С CR будет как минимум два человека, которые знакомы с кодом — автор и reviewer. Reviewer не знает код так, как его знает автор, но он знаком со структурой и архитектурой кода, что очень значимо.

Конечно же, это не просто. Из моего опыта могу сказать, что нужно время чтобы наладить хороший процесс CR. Я знаю несколько подводных камней, которые могут вызвать проблемы. Пока эти камни периодически вылезают CR может оставаться на плохом уровне, быть в тягость и препятствовать принятию в ежедневный workflow.

Главное правило — целью CR является нахождение проблем в коде перед тем как он будет закоммичен, т.е. правильность. Самая распространенная ошибка в CR у новичков это проверять код, основываясь на том как бы это сделал он. Обычно существует несколько путей решения одного и того же. Помимо этого, есть миллион способов написать задуманное в виде кода. Задача reviewerа, не сделать так, чтобы написанный код был похож на то, чтобы он написал, нет — он никогда не будет таким. Задача reviewerа — удостовериться в том, что код, написанный автором корректен. Когда нарушается это правило, вы в итоге придете к эмоциям и переживаниям, что не является тем, к чему вы стремитесь.

Дело в том, что это совершенно естественная ошибка. Если вы программист, когда вы смотрите на проблему в коде, вы можете сразу видеть способ ее решить и вы думаете, что то, что вы увидели и есть решение. Но это не так, и чтобы стать хорошим reviewerом нужно это понять.

Второй по значимости подводный камень то, что люди чувствуют необходимость сказать что-либо. Вы знаете, что автор потратил достаточно времени и сил, работая над кодом и вроде как вы должны высказать свое мнение.

Нет, вы не должны.

Вполне нормально просто сказать «Выглядит хорошо». Если вы постоянно пытаетесь найти что-нибудь покритиковать, тогда все чего вы добьетесь так это подорвать свой авторитет и поломать отношения в команде. Когда вы критикуете код только для того чтобы найти что сказать, тогда люди чей код вы проверяете поймут, что вы пишете только для того чтобы что-то написать и ваши комментарии не будут воспринимать всерьез.

image

Третье — скорость. Не нужно сломя голову бежать проверять только что посланный на review код, но с другой стороны нужно делать это быстро. Ваши коллеги ждут вас. Если вы и ваши коллеги не уделяете время тому, чтобы CR был сделан, причем сделан быстро, тогда CR может стать причиной недовольства людей в коллективе. Может показаться, что это переключение фокуса — взяться за CR. Это не совсем так. Не нужно бросать все в тот момент, когда новый код послан на review. Но в течение нескольких часов вы совершенно точно сделаете паузу для того, чтобы попить что-нибудь, сходить в туалет или просто походить. Когда вы вернетесь с паузы уделите внимание CR. Если вы возьмете это в привычку, то никто в команде не будет подолгу ожидать ваш review и он не будет доставлять никакого дискомфорта в работе.

Дополнительные материалы:
Tags:
Hubs:
+32
Comments 32
Comments Comments 32

Articles