Бывает задача перед релизом: форма открывается, документ проводится, отчет что-то показывает, обмен на тестовом примере отработал. Вроде можно закрывать.
Потом открываешь код и видишь запрос внутри цикла. На пяти строках табличной части всё летает. На рабочей базе с несколькими сотнями строк пользователь потом скажет: “После обновления форма стала зависать”.
Или видишь серверный вызов из формы при каждом изменении строки. Или проверку, которая работает только под пользователем с полными правами. Или доработку типового сценария, где разработчик честно решил свою задачу, но не знал одну особенность типового функционала.
На первый взгляд всё нормально. Код не красный, форма не падает, тестовый сценарий прошел. А после релиза прилетает обращение, срочная правка, откат и неприятный вопрос: “Почему это не поймали раньше?”
Вот для этого и нужно code review.
Не для того, чтобы проверяющий показал, что он умнее. Не для того, чтобы автор кода защищался, как на допросе. Не для того, чтобы спорить, кто как привык называть процедуры.
Хорошее ревью нужно, чтобы до релиза увидеть риск: что может сломаться, замедлиться, конфликтовать с типовым функционалом, нарушить права или стать проблемой сопровождения.
У меня был опыт с обеих сторон: я проверял чужой код, и мой код тоже проверяли. И чем больше я сталкиваюсь с ревью, тем сильнее убеждаюсь: хорошая проверка должна улучшать не только задачу, но и разработчика.
Просто исправить ошибку — уже полезно. Но настоящий хороший результат получается тогда, когда разработчик понял причину замечания, разобрался глубже, посмотрел дополнительный материал и в следующей задаче сам увидел похожий риск.
Вот это уже не просто code review. Это нормальное развитие команды.
Почему ревью так легко становится личной историей
Код — это сделанная работа. Разработчик потратил время, разобрался в задаче, что-то проверил, возможно, уже устал от согласований и хочет наконец закрыть доработку.
И тут приходит проверяющий с комментариями.
Если комментарий звучит как “переделай, так плохо”, автор кода редко думает: “Отлично, сейчас я профессионально вырасту”. Чаще он думает: “Опять придираются”.
Это нормальная человеческая реакция. Особенно если замечание написано коротко, резко и без объяснения.
Проблема не в самом факте проверки. Проблема в том, что ревью часто формулируют так, будто проверяют не код, а профессиональную пригодность человека:
- “Так нормальные разработчики не пишут.”
- “Зачем ты вообще так сделал?”
- “Мне не нравится.”
- “Переделай нормально.”
- “Это же очевидно.”
После таких фраз обсуждение быстро уходит от технического риска к эмоциям. Автор начинает защищаться. Проверяющий начинает доказывать. Код становится поводом, а не предметом разговора.
Я стараюсь держать другой принцип: на ревью мы проверяем не человека, а последствия его решения.
Это особенно важно. Здесь проблема не всегда выглядит как явная ошибка компиляции. Код может быть синтаксически правильным, задача может проходить на тестовом примере, но в реальной базе появятся нюансы:
- у пользователя нет нужных прав;
- в документе не пять строк, а пятьсот;
- типовой обработчик срабатывает не так, как ожидали;
- обмен повторно загрузил те же данные;
- регламентное задание запустилось параллельно;
- ошибка проглочена, а сопровождение потом ищет проблему вслепую.
Вот это и нужно обсуждать на ревью. Не “ты плохо сделал”, а “здесь есть риск, давай посмотрим”.
Ревью без токсичности — это не мягкость
Иногда фразу “без токсичности” понимают неправильно. Как будто проверяющий должен писать только мягко, осторожно и обходить острые углы.
Нет.
Если код может уронить сценарий, создать дубли, нарушить права или сломать типовое поведение, это нужно сказать прямо. Ревью без токсичности — это не про то, что все друг друга гладят по голове. Это про то, что мы говорим жестко по сути, но не бьем по человеку.
Можно написать:
“Ты опять не проверил права.”
А можно написать:
“Сценарий сейчас проверен под пользователем с полными правами. У обычного пользователя после релиза может упасть чтение справочника или запись документа. Это блокер: нужно проверить под реальной ролью.”
Смысл строгий. Замечание обязательное. Но во втором варианте есть техническая причина, риск и следующий шаг. Автор понимает, что исправлять и зачем.
Для меня это принципиальная разница.
Главный принцип: замечание должно объяснять последствия
Хорошее замечание на ревью состоит не из команды “исправь”, а из понятной цепочки:
- где именно место в коде;
- что в нем смущает;
- почему это может стать проблемой;
- что лучше сделать;
- насколько это критично для релиза.
Например, плохой комментарий:
“Опять запрос в цикле. Переделай.”
По сути замечание может быть правильным. Но написано оно так, что автор кода слышит не технический аргумент, а раздражение.
Лучше так:
“Здесь запрос выполняется внутри цикла по строкам табличной части. На небольшом документе это незаметно, но на большом объеме строк форма начнет заметно тормозить. Лучше собрать список ссылок и получить данные одним запросом.”
Во втором варианте разработчик понимает причину. Не просто “проверяющий так захотел”, а есть понятное последствие: нагрузка, тормоза, реальный пользовательский сценарий.
Именно это отличает полезное ревью от вкусовщины. Если замечание нельзя объяснить через риск, поддержку, производительность, права, возможно, это не блокер, а личное предпочтение.
Замечание “мне не нравится” бесполезно, пока не объяснен риск.
Ревью как развивающая обратная связь, а не список придирок
Я не считаю ревью только технической проверкой. Для ведущего разработчика, тимлида или наставника это еще и способ развивать людей.
Можно оставить комментарий “исправь права”. Разработчик исправит конкретное место и пойдет дальше.
А можно объяснить: “Сейчас сценарий проверен под пользователем с полными правами. После релиза обычный пользователь может получить ошибку при чтении справочника или записи документа. Нужно проверить роль, под которой реально работает пользователь”.
Во втором случае разработчик забирает себе не только правку, но и принцип: проверять не только под собой, а под реальным профилем пользователя.
Такие вещи особенно хорошо работают, потому что большая часть опыта приходит через последствия. Один раз увидел тяжелый отчет на рабочей базе — начинаешь иначе смотреть на запросы. Один раз поймал проблему прав после релиза — больше не ограничиваешься проверкой под полными правами. Один раз получил дубли из-за повторного запуска обмена — начинаешь заранее думать про внешний идентификатор и безопасный повтор.
В идеале ревью дает разработчику этот опыт без падения прода.
Пример: код аккуратный, а риск реальный
Не все проблемы на ревью выглядят как “плохой код”. Иногда код написан аккуратно, но разработчик не знает контекст типового функционала.
Например, разработчик меняет логику заполнения документа и проверяет новый сценарий. Новый сценарий работает. Документ создается, поля заполняются, пользовательский кейс проходит.
На ревью выясняется, что старый типовой сценарий использует этот же обработчик при другом способе создания документа. Если это не проверить, после релиза часть документов начнет заполняться иначе. Ошибка не синтаксическая, код вроде нормальный, разработчик задачу понял правильно. Но риск реальный.
Именно здесь ревью особенно полезно. Проверяющий видит не только конкретный фрагмент кода, но и место этого фрагмента в общей системе: типовой механизм, старые документы, соседние доработки, права, обновления, поддержку.
У меня были случаи, когда ревью помогало поймать проблему именно из-за такого свежего взгляда. Разработчик сделал решение логично в рамках своей задачи, но не знал одну особенность типового функционала. На ревью это заметили, обсудили, поправили. В релиз ушло более безопасное решение.
Были и обратные ситуации. Задачу посмотрели недостаточно внимательно или вообще не стали глубоко проверять, потому что “там правка небольшая”. После релиза проблема проявилась уже на рабочей базе. Доходило до срочного отката изменений. В такие моменты особенно хорошо понимаешь, что ревью — это не бюрократия, когда оно применяется к рискованным задачам. Это страховка от пожара.
Что действительно стоит смотреть в коде
Я не люблю огромные чек-листы, которые прикладывают к любой задаче. Они быстро превращаются в формальность: галочки стоят, а суть никто не смотрел.
Сам по себе список вопросов не спасает. Можно проставить все галочки и всё равно пропустить место, которое уронит сценарий после релиза. На ревью важнее понимать риск конкретной доработки.
Но есть зоны, где ошибки особенно часто становятся проблемой после релиза.
| Что смотрим | На что обратить внимание | Какой риск ищем |
| Запросы | Запросы в цикле, отсутствие отбора по периоду, лишние соединения, непонятный объем данных | На тестовой базе быстро, на рабочей базе отчет или форма начинают тормозить. Потом это выглядит не как “ошибка запроса”, а как “после обновления всё стало медленнее” |
| Клиент-сервер | Лишние серверные вызовы из формы, бизнес-логика на клиенте, частые обращения при изменении строк | Интерфейс тормозит, код сложно переиспользовать, поведение зависит от формы |
| Права | Проверка под обычным пользователем, роли, ограничения доступа, чтение и запись объектов | У разработчика с полными правами всё зеленое, а утром после релиза обычный пользователь ловит ошибку доступа |
| Типовой функционал | Влияние на стандартные сценарии, проведение, заполнение, расширения, обработчики | Доработка решает одну задачу, но ломает уже существующий процесс |
| Документы | Проведение, отмена проведения, перепроведение, старые документы, закрытые периоды | Ошибка проявится не сразу, а в учете, отчетах или при закрытии периода |
| Формы | Смешение интерфейса, бизнес-логики и записи данных в обработчиках формы | Код становится трудно сопровождать, повторно использовать и безопасно менять |
| Обмены | Внешний идентификатор, безопасный повтор, обработка ошибок, логирование | Повторная загрузка создает дубли или скрывает причину сбоя. В журнале тишина, а пользователи уже видят неправильные данные |
| Регламентные задания | Защита от параллельного запуска, лог ошибок, поведение при зависании | Фоновая ошибка накапливает последствия, пока ее не заметят пользователи |
| Сопровождение | Понятна ли цель доработки, есть ли связь с задачей, не осталось ли “временно” без объяснения | Через полгода никто не понимает, зачем это сделано и можно ли трогать |
Возьмем для примера три зоны, на которые стоит обращать внимание: запросы, права и соотношение доработки с типовым функционалом и соседними изменениями.
Запросы часто кажутся безобидными, пока данных мало. Запрос в цикле, отчет без ограничения периода, тяжелая СКД без понимания объема — всё это может не проявиться на тестовом примере. Но пользователь работает не на демонстрационных пяти строках.
Права часто проверяют слишком поздно. Разработчик тестирует под собой, администратор тестирует под собой, а потом обычный пользователь получает ошибку в рабочий день. На ревью стоит прямо задавать вопрос: “Под какой ролью это проверено?”
Типовой функционал — отдельная боль 1С. Иногда код выглядит аккуратно, но вмешивается в сценарий, который уже давно работает. Здесь важно смотреть не только на написанную процедуру, но и на то, как она встраивается в общий процесс.
Не каждое замечание нужно превращать в обязательное исправление
Одна из причин токсичности на ревью — проверяющий подает любое свое замечание как блокер.
В итоге автор кода получает длинный список и не понимает, где реальная опасность, а где проверяющий просто предпочитает другой стиль. Это раздражает и снижает доверие к ревью.
Я разделяю замечания по уровню.
| Уровень | Когда использовать | Пример для 1С |
| Блокер | Нельзя выпускать, пока не исправлено | Нарушение прав, риск дублей, падение типового сценария, ошибка записи данных, опасное изменение проведения документа |
| Важно | Лучше исправить до релиза, потому что риск заметный | Запрос в цикле, лишние серверные вызовы из формы, тяжелый отчет, отсутствие логирования в критичном обмене |
| Рекомендация | Улучшает код, но не всегда должна блокировать задачу | Уточнить имя процедуры, вынести повторяющийся код, добавить поясняющий комментарий, упростить длинный обработчик |
| Вопрос | Нужно понять намерение автора | Почему выбран этот механизм, какие сценарии проверены, что будет при повторном запуске, почему не использован типовой вариант |
| Вкусовщина | Личное предпочтение без стандарта и понятного риска | Переименовать только потому, что проверяющему привычнее; переписать рабочий код в личном стиле; спорить о форме без влияния на сопровождение |
Такое разделение сильно снижает напряжение. Автор кода видит, что от него не требуют “переписать всё, потому что проверяющий так сказал”. Есть критичные вещи, есть рекомендации, есть вопросы для обсуждения.
Проверяющему это тоже помогает. Перед тем как написать комментарий, он сам задает себе вопрос: “Это блокер? Важное замечание? Рекомендация? Или я просто хочу сделать по-своему?”
Где заканчивается риск и начинается вкусовщина
Вкусовщина на ревью появляется незаметно.
Сначала проверяющий действительно смотрит риски. Потом видит кусок кода и думает: “Я написал бы иначе”. Это нормальная мысль. Опытный разработчик почти всегда видит несколько вариантов решения.
Но мысль “я написал бы иначе” сама по себе еще не замечание.
Чтобы она стала полезным комментарием, нужно объяснить, почему текущий вариант опасен или хуже для сопровождения. Если объяснения нет, лучше не делать из этого обязательную правку.
Примеры нормальной границы:
- Название процедуры не соответствует стандарту команды — замечание предметное.
- Название процедуры просто не нравится проверяющему — рекомендация или вкусовщина.
- Код в обработчике формы смешивает запись данных и интерфейсную логику — предметное замечание.
- Проверяющий хочет разложить код на процедуры ровно так, как привык сам — повод для обсуждения, но не всегда блокер.
- Запрос без отбора по периоду может тормозить на рабочей базе — предметное замечание.
- Проверяющему не нравится порядок условий, но на читаемость и стандарт это не влияет — вкусовщина.
Если замечание нельзя объяснить через последствия, производительность, релизный риск или стандарт команды, не стоит выдавать его за обязательное исправление.
Плохое замечание и нормальная обратная связь
Мне ближе термин “обратная связь”, чем “замечание”. Замечание часто воспринимается как красная тряпка. Обратная связь звучит спокойнее: мы смотрим работу и помогаем сделать ее безопаснее.
Смысл не в том, чтобы смягчить любую техническую проблему до состояния “может быть, когда-нибудь”. Блокер остается блокером. Но даже блокер можно сформулировать так, чтобы человек понял риск, а не почувствовал унижение.
| Как писать не стоит | Как писать лучше |
| Переделай, так плохо. | Здесь запись данных находится прямо в обработчике формы. Если эту логику понадобится использовать из другого места, ее придется копировать или снова дергать форму. Лучше вынести запись в серверную процедуру, а в форме оставить вызов. |
| Опять запрос в цикле. | Запрос выполняется для каждой строки табличной части. На большом документе это даст много обращений к базе. Лучше получить данные одним запросом по списку ссылок. |
| Права не проверил. | Сейчас видно, что сценарий проверялся под широкими правами. Нужно проверить роль обычного пользователя: чтение, запись и доступ к форме. Иначе ошибка может проявиться только после релиза. |
| Так не делают. | У регламентного задания нет защиты от повторного запуска. Если предыдущее выполнение зависнет или пойдет дольше обычного, может начаться параллельная обработка тех же данных. |
| Странный костыль. | Решение выглядит временным, но в коде нет причины, срока и владельца. Такие места обычно остаются надолго. Лучше зафиксировать, зачем это сделано и когда нужно убрать. |
| Мне не нравится название. | Название не показывает, что процедура меняет состояние документа. При сопровождении это легко пропустить. Лучше назвать так, чтобы из имени было видно действие. |
В хорошей обратной связи есть не только “что исправить”, но и “зачем”. Именно это делает ревью развивающим.
Как автору кода не уходить в защиту
Ревью требует зрелости не только от проверяющего. Автору кода тоже нужно учиться принимать обратную связь.
Это не всегда приятно. Особенно когда задача сложная, сроки поджимают, а проверяющий нашел проблему в месте, которое казалось уже готовым. Но неприятное ощущение еще не значит, что замечание плохое.
Я стараюсь разделять две вещи: тон комментария и техническую суть. Тон может быть неидеальным, но внутри может быть реальный риск. И наоборот: комментарий может быть вежливым, но по сути вкусовым.
Автору кода помогают простые вопросы:
- Какой риск здесь видит проверяющий?
- Это блокер или рекомендация?
- Какой сценарий может сломаться?
- Проверялся ли этот код под обычным пользователем?
- Есть ли связь с типовым функционалом или другой доработкой?
- Можно ли оставить решение сейчас, а улучшение вынести отдельно?
Если замечание непонятно, нормально попросить объяснить:
“Подскажи, пожалуйста, какой риск здесь основной? Производительность, права или сопровождение?”
Или:
“Я выбрал этот вариант из-за ограничения типового механизма. Давай проверим, не ломает ли он соседний сценарий.”
Так разговор остается техническим. Не “я прав — ты неправ”, а “какие последствия у решения”.
Когда разработчик воспринимает ревью не как личное нападение, а как дополнительный взгляд на работу, он растет быстрее. Особенно если проверяющий не просто пишет “исправь”, а объясняет причину.
Как проверяющему не превращать ревью в самоутверждение
У проверяющего есть власть. Особенно если он ведущий разработчик, тимлид или наставник. Его комментарий может быть воспринят как финальное решение, даже если он написал его на эмоциях или из личной привычки.
Поэтому проверяющему важно следить не только за кодом, но и за способом общения.
Я держу для себя несколько правил.
- Писать про код, а не про человека. Не “ты опять забыл”, а “в этом месте не проверен сценарий обычного пользователя”.
- Объяснять последствия. Не “так нельзя”, а “после повторного запуска можем получить дубли”.
- Не использовать сарказм. Он почти никогда не улучшает код, зато быстро портит доверие.
- Не блокировать задачу личным стилем. Если нет стандарта и нет риска, это рекомендация, а не обязательная правка.
- Давать направление исправления. Разработчику полезнее понять подход, чем просто получить запрет.
- При повторяющихся ошибках учить, а не бесконечно ругаться. Если команда постоянно делает запросы в цикле, нужен общий пример или короткий разбор, а не десятый раздраженный комментарий.
Наставническая роль на ревью проявляется именно здесь. Можно “прибить” человека замечаниями. А можно так объяснить проблему, что он сам захочет разобраться и улучшить код.
Второй вариант сложнее. Зато он работает дольше.
Взаимное ревью и свежий взгляд
Ревью не обязано быть движением только сверху вниз. Иногда полезный вопрос задает не самый старший разработчик, а человек, который просто смотрит на код свежими глазами.
Автор задачи часто привыкает к своему решению. Он уже знает, почему здесь такой отбор, зачем этот обработчик, почему выбрано расширение, откуда берется значение, почему проверка стоит именно здесь. Для него это очевидно.
Для другого человека — нет.
И это хорошо. Потому что код потом будет читать не только автор. Его будет сопровождать команда. Через месяц, через полгода, после обновления, после срочного обращения пользователя.
На ревью полезны не только утверждения, но и вопросы:
- “Что будет, если документ перепроведут?”
- “Почему здесь не используем типовой механизм?”
- “Что будет при повторном запуске обмена?”
- “Какая соседняя доработка может затронуть это место?”
- “Нужно ли зафиксировать причину такого решения в описании задачи?”
Иногда после такого вопроса код не меняется. Но появляется понимание, что решение осознанное. Иногда вопрос вскрывает проблему, которую автор просто не увидел. Оба варианта полезны.
Как не сделать ревью бюрократией
Есть другая крайность: превратить ревью в тяжелый обязательный ритуал для каждой мелкой правки.
Поменяли подпись на форме — полноценное ревью. Поправили видимость кнопки — длинный список проверок. Изменили текст сообщения — несколько кругов согласования. В такой команде люди быстро начинают воспринимать ревью как тормоз, а не как помощь.
Я не считаю, что любую задачу нужно проверять одинаково глубоко. Глубина ревью должна зависеть от риска.
Если правка маленькая и не влияет на данные, процесс и безопасность, ревью может быть коротким. И это нормально.
Полезный ориентир:
- рискованные задачи проверяем глубже;
- типовой функционал и соседние доработки смотрим особенно внимательно;
- мелкие безопасные правки не превращаем в недельное согласование;
- рекомендации не выдаем за блокеры;
- повторяющиеся замечания превращаем в стандарт, пример или обучение;
- не спорим каждый раз о том, что уже договорились считать стандартом команды.
Ревью должно помогать выпускать изменения спокойнее, а не создавать отдельную очередь страданий перед релизом.
Короткий ориентир для проверки 1С-кода
Полноценный чек-лист на три страницы я здесь не даю. В этой теме важнее мышление, чем галочки. Но короткий ориентир перед ревью полезен.
- Понятна ли цель изменения?
- Не ломается ли типовой или старый пользовательский сценарий?
- Проверены ли права обычного пользователя?
- Нет ли запроса в цикле?
- Есть ли отборы в запросах, особенно по периоду и организации, если они нужны?
- Нет ли лишних серверных вызовов из формы?
- Не смешаны ли интерфейс, бизнес-логика и запись данных в одном обработчике?
- Понятно ли, что произойдет при ошибке?
- Не скрываются ли исключения без следа?
- Есть ли защита от повторного запуска для обмена или регламентного задания?
- Не конфликтует ли изменение с другой доработкой?
- Можно ли сопровождать этот код через полгода?
Если по какому-то пункту появляется тревога, это не повод нападать на автора. Это повод нормально обсудить риск.
Ревью помогает не только релизу, но и сопровождению
Хорошее ревью часто окупается не в день проверки, а позже.
Когда релиз прошел без пожара. Когда пользователь с обычными правами смог выполнить сценарий. Когда отчет не повис на рабочей базе. Когда другой разработчик через полгода открыл доработку и понял, зачем она была сделана.
В 1С сопровождение часто становится дороже самой разработки. Особенно если в базе накоплены быстрые решения без объяснений: временные обработки, непонятные расширения, обработчики формы на сотни строк, права “потом проверим”.
Ревью не спасет от всех старых проблем. Но оно помогает не создавать новые.
Для этого проверяющий должен смотреть шире, чем “код компилируется” и “на моем тесте работает”. Нужно задавать вопросы:
- что будет на большом объеме данных;
- что будет у пользователя без полных прав;
- что будет при повторном запуске;
- что будет со старыми документами;
- что будет при обновлении типовой конфигурации;
- что будет, если рядом появится еще одна доработка;
- что будет, когда этот код откроет другой разработчик.
Именно эти вопросы обычно важнее спора о том, “как красивее”.
Финальный вывод
После плохого ревью у человека остается ощущение, что к нему придрались. После хорошего — понимание, почему место опасное и как в следующий раз увидеть такой риск самому.
Разница вроде небольшая: и там, и там проверяющий написал комментарий. Но для команды разница огромная. В первом случае разработчик защищается и старается быстрее закрыть обсуждение. Во втором — растет профессионально и начинает сам смотреть на код шире.
Code review без токсичности — это не разговор про мягкость. Иногда замечание должно быть жестким по сути: нельзя выпускать код, который ломает права, создает дубли, роняет проведение или конфликтует с типовым процессом.
Но даже жесткое замечание можно написать спокойно и предметно.
На ревью важно проверять не человека, а последствия его решения. Что сломается? Где затормозит? Какие права не учтены? Как это будет сопровождаться? Не конфликтует ли это с типовым функционалом или соседней доработкой?
Если проверяющий объясняет риск, а автор кода учится принимать обратную связь без защиты каждой строки, ревью начинает работать правильно. После него становится лучше не только конкретная задача. Сильнее становится вся команда.
Для меня хороший результат ревью — это не только исправленный запрос, вынесенная серверная процедура или добавленная проверка прав. Хороший результат — когда разработчик понял принцип и в следующей задаче сам увидел этот риск раньше.
Хорошее code review — это не способ показать, кто умнее. Это способ найти риск до релиза, сохранить сопровождаемость и договориться о качестве без личных войн.
Вступайте в нашу телеграмм-группу Инфостарт