Google: Handling pushback in code reviews
Google, основываясь на многолетнем опыте, представил свои стандарты того, как лучше всего проводить code review. Все вместе они представляют собой один законченный документ, разбитый на множество отдельных разделов. Это перевод стандартов по проведению Code Review от Google. Вот ссылка на оригинал!
Терминология:
CL — расшифровывается как «список изменений» (“ChangeList”), что означает одно автономное изменение, которое было отправлено в систему управления версиями или подвергается проверке кода. Другие организации часто называют это «изменением» или «патчем».
Преодоление давления в процессе code review
Иногда разработчик может спорить в процессе code review. Либо он не согласен с вашим предложением, либо недоволен тем, что вы слишком строги.
Кто прав?
Если разработчик не согласен с вашими предложениями, то стоит проверить, действительно ли они верны. Зачастую разработчики ближе к коду, чем вы, и поэтому они действительно могут лучше знать некоторые его аспекты. Есть ли смысл в аргументах разработчика? Если да, дайте ему знать, что он прав, и тогда спор сойдет на нет.
Разумеется, разработчики не всегда правы. В этом случае ревьюер должен дать дополнительные разъяснения, почему он считает, что его предложение является правильным. Грамотное объяснение демонстрирует как понимание ответа разработчика, так и дополнительную информацию о том, почему требуется изменение.
В частности, когда ревьюер полагает, что его предложение улучшит качество кода, он должен продолжать настаивать на нем, если считает, что полученное улучшение качества кода оправдывает дополнительную работу. Обычно улучшение качества кода происходит небольшими шагами.
Иногда требуется несколько раундов объяснения предложения, прежде чем оно действительно будет использовано. Главное — всегда будьте вежливы и дайте разработчику знать, что вы слышите, что он говорит, но вы просто не согласны.
Расстроенные разработчики
Ревьюеры иногда считают, что разработчик расстроится, если они будут настаивать на улучшении. Иногда разработчики действительно переживают, но обычно не долго. И впоследствие они будут благодарны за то, что вы помогли им улучшить качество их кода. Как правило, если вы вежливы в своих комментариях, то разработчики вообще не расстраиваются. Так что негативные эмоции обычно вызывает стиль написания комментариев, а не требование ревьюера к качеству кода.
Отложенные правки
Обычно причина спора заключается в том, что разработчики (по понятным причинам) хотят добиться аппрува своего CL. Они не хотят проходить еще один раунд code review. Поэтому говорят, что проведут рефакторинг в более позднем CL, и просят дать аппрув текущему CL сейчас. Некоторые разработчики довольно ответственны и сразу же сделают дополнительный CL, который решает проблему. Но опыт показывает, что чем больше времени проходит после исходного CL, тем меньше вероятность дополнительной правки. Обычно, если разработчик не сделает правки сразу после текущего CL, то не сделает их вовсе. И не потому что разработчики безответственны, а потому, что у них много других задач, и рефакторинг теряется или забывается в процессе работы. Таким образом, обычно лучше настаивать на том, чтобы разработчик очистил свой CL сейчас, прежде чем код пойдет в репозиторий. Позволять отложенные правки — это распространенный способ вырождения репозиториев.
Если CL добавляет новую сложность, то он должен быть качественным перед отправкой, если только это не чрезвычайная ситуация. Если CL добавляет некоторые проблемы, и они не могут быть решены в настоящий момент, тогда разработчик должен создать отчет об ошибке и назначить его себе, чтобы он не потерялся. Также можно написать комментарий TODO в коде, который ссылается на зарегистрированную ошибку.
Жалобы на строгость
Если ранее вы проводили поверхностные code review, а после стали делать более строгие проверки, то некоторые разработчики будут недовольны. Но повышение скорости проверки кода обычно приводит к исчезновению подобных жалоб.
Иногда могут потребоваться месяцы, чтобы такие жалобы исчезли, но в конечном итоге разработчики склонны видеть ценность строгих проверок кода, поскольку они видят, какой отличный код получается в итоге. Зачастую самые активные протестующие даже становятся вашими сторонниками, когда они видят ценность строгих проверок.
