Культура проверки кода и лучшие практики PR

Проверка кода — это не способ контролировать доступ, а возможность команде учиться вместе, передавать ответственность и поддерживать высокое качество. Хорошая культура проверки помогает всей команде развиваться, а плохая создаёт узкие места и вызывает недовольство.

1) Делайте его небольшим — по возможности менее 400 строк. 2) Пишите понятное описание: зачем нужны изменения, что они делают и как их проверить. 3) Добавляйте ссылку на задачу. 4) При изменениях интерфейса добавляйте скриншоты или видео. 5) Самостоятельно проверьте изменения перед тем, как запросить проверку.

Используйте те же стандартные префиксы, что и в коммитах: feat: add user profile page, fix: handle 404 in fetch wrapper, refactor: extract Avatar component. Многие команды создают журналы изменений на их основе.

Большинство команд используют шаблон PR — установите его в .github/pull_request_template.md.

Перед тем как запросить проверку, пройдите по собственным изменениям строка за строкой. Добавьте комментарии, объясняющие неочевидные решения. Часто Вы сами обнаружите ошибки ещё до того, как их придётся искать кому-то другому.

PR на 2000 строк редко проверяют тщательно. Разделите его на: 1) рефакторинг без изменения поведения, 2) добавление нового поведения, 3) улучшение интерфейса. Каждую часть будет проще проверить и отменить.

Формулируйте замечания как вопросы, а не приказы: «Что Вы думаете о том, чтобы вынести это в хук?» лучше, чем «вынесите это». Разделяйте обязательные исправления и желательные улучшения. Используйте префиксы: nit:, question:, blocker:.

Фраза «Это непонятно» ничего не говорит автору. «Мне пришлось прочитать это три раза, чтобы понять досрочный выход. Может, вынесем его в защитное условие?» — уже конкретное предложение, с которым можно работать.

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

Prettier отвечает за форматирование. ESLint отвечает за стиль. Не тратьте время проверки на споры о табах и пробелах. Если одно и то же правило стиля постоянно обсуждается, зафиксируйте его в анализаторе кода.

Тесты — тоже код. Убедитесь, что у нового кода есть тесты. Проверьте, что тесты действительно проверяют нужное поведение: многие тесты проходят даже при сломанном коде, потому что проверяют не то, что нужно.

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

Проверяйте изменения в течение одного рабочего дня. В давно не обновляемых PR теряется контекст: автор уже переключился на другие задачи, а ветку нужно перебазировать. Большие PR, которые лежат без движения неделю, всегда превращаются в мучительное слияние.

Функция предложений GitHub позволяет автору принять исправление одним нажатием. Это гораздо быстрее, чем писать в обычном тексте «замените эту строку на X».

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

Как следует относиться к замечаниям при проверке кода, если Вы написали бы его иначе?

Автор: небольшие, хорошо описанные PR со скриншотами и тестами. Сначала проверьте изменения самостоятельно. Проверяющий: конструктивные замечания в форме вопросов. Разделяйте обязательные исправления и мелкие пожелания. Отмечайте хорошие решения. Не тратьте время на стиль — пусть им занимаются инструменты. Проводите проверку в течение дня. Одобряйте изменения только тогда, когда понимаете их. Шаблоны PR делают процесс единообразным. Проверка — это совместная работа, а не контроль доступа.