Как завести у себя в команде код-ревью. Отвечаем на вопросы

17.07.19

Разработка - Рефакторинг и качество кода

Дадим советы как начать использовать у себя в команде код-ревью (code-review), а также ответим на вопросы читателей.

В рамках предыдущей статьи "По следам код-ревью" меня попросили написать поподробнее про инспекцию кода в своей команде: 

Интереснее как такое внедрить в команде: 
- кто проводит 
- с какой периодичностью 
- как исправляете (задачу создаете или еще как) 
- как контролируете выполнение исправления (вдруг разработчик просто забил на замечание) 
- как боретесь с вкусовщиной (когда у разных разработчиков разное понимание стандартов) 
- всех проверяете или выборочно

и

Но про организационные аспекты было бы не менее интересно узнать. 
На каких проектах код-ревью целесообразно применять? 
Какие трудности возникают? 
Как долго применяете? 
Сколько времени тратится и каких специалистов? 
Очень полезно было бы узнать ответы на эти вопросы.

и другие. 

У меня появилось немного свободного времени, так как троих мелких я развез по бабушкам на текущий месяц, то выполняю свое обещание. Рекомендации базирую на своем опыте, выражаю свое мнение и результаты размышлений. Саму статью я решил сделать в формате вопрос и ответ.

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

Во-вторых, этот процесс внедрения является самым настоящим проектом, поэтому у него должен быть руководитель – тот человек, который начнет и завершит его. Само собой, это не поедет.

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

Если по первым пунктам вы готовы, тогда поехали!

 

Что такое код-ревью?

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

Процесс ревью - это процесс контроля того что задача выполнена в полном объеме, соблюдены все правила и договоренности, что решение не избыточно, что его легко поддерживать и развивать в дальнейшем.

 

Что нам дает код-ревью?

 
Разработка без code-review и с code-review.

Нам обзор кода привнес следующие бонусы (простыми словами):

  1. Это мощная проверка того что делает джун или новичок.
    Ему становится проще настроиться на одну волну или получить опыт. 
  2. Позволяет отсечь ошибки.
    Быстро находятся глупые ошибки и опечатки. Хочу заметить, тут нет цели найти все 100% ошибок (это вопрос тестирования), нашли несколько уже здорово. 
  3. Повышается ответственность.
    Человек, который знает, что его будут проверять, уже пишет лучше.  
  4. Повышаем свою квалификацию
    После многократного разбора ошибок и методик "проблемного" кода стало значительно меньше, и соответственно качества кода выше.
  5. Уменьшается эффект bus factor (эффект автобуса). 
  6. Позволяет выбрать наиболее лучшее решение.
    Проверяющий (щие) может не согласиться с реализацией и тогда проблема эскалируется и формируется коллегиальное обсуждение.
  7. Снижаются временные затраты на исправления ошибок и необходимости в доработках после релизов, а также уменьшаются репутационные потери.
  8. Общий код выглядит лучше
    В коде становится меньше "макарон".
  9. Повышается степень совместного владения кода.
    Команда мыслит на одной волне. Появляется согласованность в принятии тех или иных решений.

 

Что требуется для внедрения методики код ревью в процесс разработки?

Ответ: 

  1. Желание или необходимость.
  2. Система баг-трекинга.
    Это то место, где проходит основная коммуникация в рамках процесса разработки. По моему мнению, без этого инструмента говорить про процесс разработки бессмысленно.
  3. Бизнес-процесс разработки.
    Описание процесса жизни задачи или по крайней мере понимание что и зачем и кто, и что делает. Статусы и описание назначения и процессов, происходящих в этом статусе.
  4. Команда.
    Если в команде 1 человек, то данную методику применить невозможно.

 

Как внедрить код-ревью?

Ответ:

  1. Понимаете, что вы этого хотите. Что вы «созрели». Что вы готовы это сделать?
  2. Добавляете в свой процесс новый статус: «код-ревью» и/или «требуется код-ревью».
  3. Описываете как задачи попадают в этот статус, что требуется сделать в этом статусе, что потом следует.
  4. Определяетесь с ответственными по проведению код-ревью.
  5. Определяетесь с внутренними стандартами. Согласуете определенный стиль и подход.
  6. Определяетесь как будет происходить коммуникация в рамках проведения код ревью, как что и где смотреть.
  7. Определяетесь, когда по времени будет проводиться код-ревью.
  8. Начинаете работать.

По части тезисов будут расшифровки ниже по тексту. 
 


Дорожная карта от желания «хочу внедрить код-ревью» до успешного использования в команде

 

Кто может выполнять код-ревью?

Ответ:

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

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

Замечания:

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

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

Задачи на код-ревью выдаются на команду и может взять кто-то любой? Это здорово, но всегда будет какая-то сложная задача или сложный автор, которого будут всячески избегать, и еще если задачка не очень важная. Да, еще все будут думать друг на друга: Вася сделает, а Вася будет думать, что пусть Петя возьмет. При таком подходе у нас на ревью задача как-то висела месяц, она была еще и не особо важная (низкий приоритет). 

Должен быть назначен один ответственный по код-ревью? А вот дальше можно меняться задачками друг с другом, совещаться, призывать коллективный разум.

 

Нужен ли кто-то для контроля?

Обязательно должен быть ответственный человек, который все этим управляет и контролирует (по крайней мере на старте). Если не будет ответственного, то это все дело обязательно скатится в хаос и нечего не получится.
 

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

Ответ:

Привожу пример процесса с использованием инспекции кода:

Задача принимается в работу, а после отправляется в соответствующий статус, который требует проведения обзора кода (можно провести аналогию с pull-request).

Далее ответственное лицо принимает задачу в работу (статус "Код-ревью") и проверяет. Результатом его работы будет принятие задачи или возврат на доработку (более подробно процесс взаимодействия с примерами см. "Процесс проведения код-ревью?"). 

На рисунке приведена доска SCRUM (см. ниже "Пример доски задач SCRUM"), которая содержит в себе представление процесса разработки.

В колонке "Код-ревью" находятся (объединены) задачи с двумя статусами "требуется код-ревью" и "код-ревью" - это колонка инспектора по коду).

В колонке "В работе" объединение статусов "В работе" и "Возврат" - это колонка разработчика.

 
Пример доски задач SCRUM

 

Процесс проведения код-ревью? 

Ответ:

Мы проводим код-ревью в рамках бизнес процесса. Если задача не прошла код ревью, тогда она возвращается в работу. И после завершения опять переводится на код-ревью.

Наличие подтвержденного код-ревью у нас проверяется при формировании сборки.
Результатом код-ревью в задаче должна быть отдельная отметка [КОД-РЕВЬЮ] с информацией о том, что она пройдена.

[КОД-РЕВЬЮ]
ОК. 

В случае наличия замечаний, то автору приводится краткий список проблем:

[КОД-РЕВЬЮ]
1.    Изменить форматирование
2.    Используем запросную модель, а не объектную в модуле объекта….
3.    Возможна ошибка при использовании…

Автор при возврате отвечает оппоненту:

1.    Сделал
2.    Сделал
3.    Ошибки нет, т.к.

Как погасить эскалацию конфликта ревьювера и автора?

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

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

 

Что такое дизайн-ревью (desing-review)?

Можно сказать так  - это проверка архитектуры кода предлагаемой разработчиком. Ее рекомендуется выполнять до начала процесса кодинга, после принятия задачи в работу и проведения первичного анализа. Можно собраться группой и выслушать предложения разработчика о выбранном им проектном решении, либо совместно обсудить какое решение лучше принять. Для проведения этого обзора лучше всего привлечь в "комиссию" самых опытных разработчиков и/или архитектора.   

Благодаря использованию desing-review вы многократно усиливаете эффект code-review. Тем самым вы устраняете ошибку выбора проектного решения, до момента когда ее устранить довольно сложно или уже не возможно.

Для разработчика вы разбиваете задачу на два этапа:

  1. "я понимаю, что и как собираюсь делать" 
  2. "я делаю"

Когда вы выполняете этот пункт то необходимо ответить на следующие вопросы:

Можно сделать проще  и удобнее? 
Нет ли избыточности? 
Соответствует ли задаче реализация?
Это костыль или нет?
Используются типовые решения или это новый велосипед?
Соответствует ли паттерну открытости/закрытости (если соответствует, то при доработках не потребуется переписывать все решение снова, а только добавлять новый код)?

Что смотрим при проведение инспекции кода?

Можно сформулировать в общем случае следующий набор вопросов на которые должен ответить инспектор кода:

Соответствует ли код стилю? Соблюдаются ли правила, стандарты и соглашения принятых в команде? (об этом ниже см. "что за стандарты и как их сделать?")
Возникают ли у меня сложности в понимании?
Можно ли что-то исправить путем рефакторинга?
Хорошие наименования переменных, функций?
Есть ли излишнее усложнение кода? Высокая ли цикломатическая сложность (фактически на сколько большая вложенность если иначе)?
Соответствуют ли запросы критериям оптимальности?

Дополнительно еще можно составить некоторый чек-лист критериев проверки, на первых порах выполнять проверять в его рамках. 

 

Что за стандарты и как их сделать?

Ответ:

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

1. Вы договариваетесь о правилах форматирования кода. 
Порядок оформления комментариев, названия переменных, создания обработок, форматирования текста и т.п.
2. Вы договариваетесь о правилах доработки типовых конфигураций. 
В нашем случае мы договорились о правилах минимального изменения конфигураций.
3. Используете правила и рекомендации по работе с БСП.
Нравится вам или нет, но к примеру, добавление нового присоединенного файла к производителям необходимо делать в соответствии с документацией от разработчиков.
4. Используете рекомендации по разработке от 1С.
5. Свои стандарты, правила и т.п.

Пример содержимого документа памятки:

А) Добавляем новые реквизиты и метаданные с префиксом ццц_
Б) Перед началом вставки кода и завершении добавляем специальные теги (используем шаблоны кода)
//++ Номер Задачи Автор
//-- Номер Задачи Автор
В) Форматирование кода используем типовое
Г) Делаем коммиты на каждую задачу отдельно и при помещении в хранилище в комментарий добавляем метку с номером задачи
Д) и т.д. …

Кого проверяем? Всех или выборочно?

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

 

Когда проводится код-ревью и как часто?

Ответ: Мы начинаем проведение ревью каждое утро с 9 часов утра, пока не закончатся задачи. 
Либо проверка может выполняться по требованию в процессе рабочего дня (срочный баг фикс и т.п.).
И конечно по желанию.

 

Как проводится контроль?

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

Перевод в статус с код-ревью выполняет только ответственный по "код-ревью".

А по факту есть определенная ответственность - отметка код-ревью должна быть. У нас это "устаканилось". Да, были разговоры и сложности в процессе, сейчас это естественно и проблем не возникают.

 

Как выгляди диалог общения внутри процесса ревью?

Ответ: Допустим только профессиональный диалог. Никаких личных отношений, переходов на «ты», высмеивания или пренебрежительного отношения.

Запомните цель code-review повысить качество вашего продукта, а не поднять настроение команде. 

Правильный и не правильный процесс code-review

 

Что пишет автор перед отправкой на код-ревью?

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

а) В задаче автор обязательно отмечает список измененных метаданных (еще несколько отдельных требований).
б) В хранилище конфигурации задачи обязательно помещаются с комментарием о номере задачи или номерах (желательно чтобы коммит был в рамках одной задачи).
в) В коде конфигурации подключенной к хранилищу разработчик отмечает начало изменения и конец изменения. Согласно стандарта (см. выше)

// ++ Задача-6800 Иванов
// комментируются данные до
// Если Объект.УчитыватьНДС Тогда
Если Объект.УчитыватьНДС и Объект.ццц_Учитывать 
….
//-- Задача-6800 Иванов

г) Если ведется разработка в EDT, то на код ревью отправляется номер ветки. Скажу честно, то в EDT сравнивать очень непривычно.

 

Как боретесь с «индивидуальными» (вкусовщиной) подходами?

Ответ: Есть общий стандарт и договоренность внутри команды. Все остальное вне закона.
Иными словами, если приходит задача и она не соответствует стандарту, то задача «сразу» отправляется в возврат.

 

Что писать в замечаниях к код-ревью?

Все замечания мы пишем в рамках тега «[КОД-РЕВЬЮ]» в нумерованном списке. Соответственно, автор при исправлении и объяснении своей позиции будет отвечать на каждый пункт из пронумерованного списка.

Если нет замечаний, то ставится резюме – «ОК». (пример оформления переписки см. пункт «Процесс проведения код ревью. Как проводится контроль?»)

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

Делаем ссылки на стандарты или правила. Все свои рекомендации, версии должны быть подкреплены доказательствами. Фразы не допустимы: "Я так считаю", "Мне нравится" и т.д.

Соответствие стандартам. Если не соответствует стандартам так и пишите: не соответствует стандартам.

 

Какие есть автоматизированные инструменты?

Здорово если бы для 1С появился мощный статический анализатор кода типа PVS-studio, но пока его нет.

Мы использовали проверку конфигурации через тестирование и исправление, типовая проверка кода. Еще можно взять автоматизированную проверку конфигурации на стандарт - 1С соответствуют.

Про остальное не знаю. Внедрять и поддерживать полу-функциональные инструменты не считаю эффективным подходом.

 

Как происходит проверка технически?

В двух словах - используем имеющиеся типовые инструменты от 1С: 

  • В конфигураторе используем обязательно хранилище и функционал сравнения истории с выборочным объектом. В данном случае удобно использовать метки в коде для выделения изменений по задаче от всего потока изменений.
  • В EDT используем функционал сравнения веток (тут мы можем сравнить даже формы). Также для сравнения кода модулей можно использовать функционал сравнения с самого GitHub (но без форм).
  • Некоторые команды используют другие инструменты для разбора конфигурации. Это совсем отдельный вопрос.

 

Сколько времени тратится на код-ревью?

Мы рекомендательно ограничиваем время на проведение процедуры код-ревью 15-20 минут на задачу. Однако на этот временной параметр влияют множество факторов: уровень автора (эксперт или джун), сложность задачи, риски и последствия плохого исполнения задачи (фактор риска), возможная эскалация и призыв коллективного разума и т.д.

Вообще не рекомендуется единоразово тратить на проведения обзора более 60 минут. А также передавать на ревью задачи с большим измененным количеством метаданных и тысячами строк кода (читал что нормально строк 200, а 500 - это максимум, дальше уже начинается треш и угар). Поэтому получается, что оптимальное количество задач на утреннюю инспекцию кода - это 3-5 средненьких задачи.

 

Как давно применяете?

Уже очень давно, минимум 5-6 лет.

 

Какие трудности возникают?

Их много особенно в начале пути.

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

Тяжелее проверять задачи в EDT, не смотря на наличие ветвлений. Тут проблемы технического характера, а также самого IDE. Разработчики от компании 1С обещали поправить, ждем.

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

Приходится всем следовать стандартам. Иногда так хочется запилить «по-быстрому», но нельзя.

Сам процесс внедрения и улучшение процесса вызывает определенные «сложности». Это, на мой взгляд, логично и понятно. А изменения должны быть, т.к. время течет и все меняется)

Заставить проводить код-ревью утром в понедельник). Поэтому мы ненавязчиво напоминаем коллегам об обязанностях.

 

Как повысить уровень, коммуникацию, эффективность внутри команды?

Ответ:

Для этого рекомендую проводить краткие вебинары (15-30 минут). Сейчас организовать подобные мероприятия не проблема – есть Skype, MS Teams, WebEx и др.

В первой части проводится ретроспектива и каждый из участников отвечает на три вопроса:

Что было плохого?
Что было хорошего?
Какие есть предложения?

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


На каких проектах целесообразно применять код-ревью?

Ответ:

Практически на всех. Не забывайте, что даже эксперт может «накосячить».

Для проектов в которых планируются изменения, доработки в будущем, будет передаваться другой команде - инспектирование кода MUST HAVE. Т.е. если проект большой (объемный по времени сопровождения, внедрения и т.п.) и на нем отсутствует код-ревью, то я считаю это преступлением против принципов хорошего тона в разработке). 

Излишне, наверное, будет инспектировать код при разработке прототипа и минимально жизнеспособного продукта (обработки, отчета на раз). Хотя и тут если особенно исполнение хромает, то лучше одним глазком посмотреть.

См. также

Когда понадобился новый оператор

Рефакторинг и качество кода Платформа 1С v8.3 Конфигурации 1cv8 Бесплатно (free)

Когда понадобился новый оператор, но его нет в синтакс-помощнике, что делать?

вчера в 08:00    676    ZhokhovM    2    

2

Когда разработчик платформы не добавил проверку препроцессоров

Рефакторинг и качество кода Платформа 1С v8.3 Конфигурации 1cv8 Бесплатно (free)

Когда разработчик платформы решил пойти на кухню за кофе, а проверку препроцессоров не добавил, и вот тут-то и началось: "Что, опять все сломалось? Ну и кофе же я забыл сделать!".😅

вчера в 07:00    1702    ZhokhovM    2    

4

Реструктуризация - бесконечная история

Рефакторинг и качество кода Платформа 1С v8.3 Бесплатно (free)

При разработке программ требуемый функционал ставят на первое место, но есть еще и архитектура программы. На горизонте 5-10 лет она становится важнее функционала, который должен работать при масштабировании и росте данных. Реструктуризация 5 терабайтной базы 1С 8.2 в формат 1С 8.3, складывает весь пазл архитектурных просчетов, которые сделали ради функционала. Как это исправить? - для разработки правильной архитектуры, нужно всего лишь сместить фокус с функционала и подумать о «вечном».

29.09.2023    1866    1CUnlimited    15    

22

Чистый код. Мой взгляд на жизнь в макаронных джунглях. Часть 2

Рефакторинг и качество кода Платформа 1С v8.3 Конфигурации 1cv8 Россия Бесплатно (free)

Коротко о том, как я перестал быть создателем макаронного кода и непроходимых джунглей методов и модулей. Расскажу о том, что реально применяю на практике с примерами при разработке (а в основном доработке) в типовых конфигурациях 1С. Комментарии очень приветствуются.

27.09.2023    6867    Lemmonbri    136    

35

Чистый код. Мой взгляд на жизнь в макаронных джунглях. Часть 1

Рефакторинг и качество кода Платформа 1С v8.3 Конфигурации 1cv8 Россия Бесплатно (free)

Коротко о том, как я перестал быть создателем макаронного кода и непроходимых джунглей методов и модулей. Расскажу о том, что реально применяю на практике с примерами при разработке (а в основном доработке) в типовых конфигурациях 1С. Комментарии очень приветствуются.

19.09.2023    4257    Lemmonbri    16    

31

5 подходов при доработке конфигурации 1С, чтобы в будущем не было мучительно больно её обновлять

Архитектура Рефакторинг и качество кода Обновление 1С Платформа 1С v8.3 Бесплатно (free)

Нашей компании часто приходится сталкиваться с обновлением конфигураций разной степени переписанности. Какие-то из них обновляются легко, какие-то — не очень. Расскажем о некоторых принципах модификации программы, которые помогут сделать последующий процесс обновления легче. Или тяжелее, если стараться их не соблюдать.

10.08.2023    9511    0    1c-izhtc    37    

21

Задача на ошибки и неоптимальности при проведении приходной накладной

Рефакторинг и качество кода Платформа 1С v8.3 Конфигурации 1cv8 Бесплатно (free)

Задачу эту дают на собеседованиях, видимо, те франчи, которые не в состоянии оценить человека по резюме и в ходе беседы. По идее задачи, подобные этой, должны давать начинающим студентам. Но дают всем подряд. Итак: мои 5 копеек. Критика приветствуется.

11.07.2023    2183    magic1s    32    

10
Комментарии
В избранное Подписаться на ответы Сортировка: Древо развёрнутое
Свернуть все
1. kuzyara 1896 17.07.19 05:27 Сейчас в теме
[КОД-РЕВЬЮ]
1. Изменить форматирование
...
Сам процесс внедрения и улучшение процесса вызывает определенные «сложности».
Неудивительно.
2. ivanov660 4325 17.07.19 07:03 Сейчас в теме
(1) Коллега есть комментарии по существу?
Или какие-нибудь дельные предложения, советы. Хотите поделиться опытом?
3. Stepa86 1520 17.07.19 08:15 Сейчас в теме
> Здорово если бы для 1С появился мощный статический анализатор кода типа PVS-studio, но пока его нет.

Я аж кофем подавился. https://infostart.ru/public/1089670/
kalyaka; JohnyDeath; olegtymko; NeviD; Iscarimet; +5 Ответить
7. ivanov660 4325 17.07.19 10:52 Сейчас в теме
(3) А от 1С есть что-то? Слишком много разных инструментов используется и для новичков это довольно напряжно будет применить и настроить.
11. Stepa86 1520 17.07.19 10:58 Сейчас в теме
(7) от 1С - АПК.

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

У нас есть конфигурация для проведения код-ревью. Там сперва каждый самостоятельно проводит анализ куска кода и пишет замечания, а потом вместе собираются и быстренько пробегаются по замечаниям с обсуждениями.
В эту конфигурацию мы подгружаем все нарушения, которые нашли Сонар+АПК+EDT и аудиторы их сразу видят. И не тратят время на них. А автор кода получает по башке за то, что сам не прогнал свой код через сонар.
user1564146; RustIG; +2 Ответить
12. ivanov660 4325 17.07.19 11:04 Сейчас в теме
(11) Не все имеют большую и продвинутую команду с хорошим набором инструментов и специалистов, как у вас.
Есть много начинающего уровня специалистов и команд, для которых на первом шаге многие вещи из вашей статьи будут непосильны. Поэтому начинать надо мелкими шагами. А вот потом, когда уже поднимется уровень, тогда можно будет смотреть в сторону вашего окружения.
Vary; RustIG; Stepa86; +3 Ответить
13. Stepa86 1520 17.07.19 11:08 Сейчас в теме
(12) Вот прям вообще ДА. Если у вас 1.5 программиста - вам не нужен инструмент по автоматическому статическому анализу кода. А если у вас большой и серьезный проект, то обязательно нужно и двойное чтение кода, и статистический анализ кода, и гит, и тесты, и покрытие, и CI/CD, и разные контуры.

Не существует единый истины. Всегда нужно головой думать в каждой конкретной ситуации. И взвешивать профит и затраты.
RSConsulting; +1 Ответить
14. ivanov660 4325 17.07.19 11:12 Сейчас в теме
(13) Может вас и удивит, но я разговаривал с коллегами на конференции, которые не были новичками и конфигурации у них были относительно большие и серьезные, но кто-то даже не использовал хранилище для разработки, что там говорить про какие-то техники. Поэтому действительно истина у каждого своя)
Vary; RustIG; +2 Ответить
15. Stepa86 1520 17.07.19 11:18 Сейчас в теме
(14) "не использовал хранилище для разработки" можно двояко понять. Есть те, кто еще не использует, а есть те, кто уже не использует.

Вы же статьи тоже пишете для того, чтоб другие почитали и улучшили процессы у себя?
16. TODD22 18 17.07.19 11:21 Сейчас в теме
(11)
А автор кода получает по башке за то, что сам не прогнал свой код через сонар.

А разве автоматически это не должно делаться?
17. Stepa86 1520 17.07.19 11:25 Сейчас в теме
(16) Не на всех проектах. Да и если прогнали - не всегда правят. У нас тут тоже как бы костыли и эксперименты, а не отлаженный идеальный процесс с розовыми понями.
18. TODD22 18 17.07.19 11:28 Сейчас в теме
(17)У нас было строго, проверка автоматически, пока не исправил код не принимают.
4. genayo 17.07.19 08:38 Сейчас в теме
"++ Задача-6800 Иванов" - не очень хороший комментарий. Имхо, вместо номера задачи более уместна прямая ссылка на треккер, и кроме задачи не помешало бы указывать проект/спринт, в рамках которого производятся изменения, чтобы потом было легко найти все изменения по проекту, сделанные всеми разработчиками.
6. CheBurator 3119 17.07.19 10:49 Сейчас в теме
(4) + по крайней мере как минимум в скобках-комментах д.б. маркер дата-время
ManyakRus; +1 Ответить
9. ivanov660 4325 17.07.19 10:55 Сейчас в теме
(6) Конечно под себя нужно "допилить"). Однако если у вас задача в системе баг-трекинга, то большой перечень комментарием может быть излишним.
25. skillman 5 21.07.19 17:29 Сейчас в теме
(9) Можете прикрепить к статье свои регламенты разработки, хотелось бы посмотреть чужую практику, возможно на будущее по взаимствовать.
27. ivanov660 4325 21.07.19 19:02 Сейчас в теме
(25)не могу, это внутренний документ, зато могу пообщаться в рамках этого вопроса.
19. genayo 17.07.19 11:41 Сейчас в теме
(6) Зачем, если есть ссылка на задачу в треккере?
5. VmvLer 17.07.19 09:37 Сейчас в теме
читал, пытался держать нить статьи в рациональном понятийном русле, в итоге пришел к мнению, что
с водой выплеснули ребенка

тут есть примеры свежих статей с хорошим стилем, где авторы просто и понятно "ведут" по теме.
не в этот раз.(
user1219528; +1 4 Ответить
8. user1219528 17.07.19 10:53 Сейчас в теме
10. ivanov660 4325 17.07.19 10:57 Сейчас в теме
(5) Коллега - я привел ответы на вопросы, которые просили. А что вы собственно хотели почерпнуть?
20. Kutuzov 736 18.07.19 09:03 Сейчас в теме
Добрый день! Хотел уточнить такой момент. Временные затраты, которые понес проверяющий - они падают на стоимость проекта? Опять же при возврате задачи на доработку программисту - исправления он делает за свой счет, или эти дополнительные часы опять же падают на стоимость проекта?
21. TODD22 18 18.07.19 09:33 Сейчас в теме
(20)
Временные затраты, которые понес проверяющий - они падают на стоимость проекта?

А на что ещё падают затраты в коммерческой организации?
ivanov660; +1 Ответить
24. Kutuzov 736 18.07.19 10:15 Сейчас в теме
(21) Ну мало ли, может это считается как инвестиции в развитие сотрудников. Я поэтому и спрашиваю, что надо узнать)
22. VmvLer 18.07.19 09:42 Сейчас в теме
(20) я полагаю, что после проверки весь отдел разработки станет полгода обучать азам тех кто наваял кривой код.
проще было бы не допускать таких к работе, но кого это волнует.
сейчас принято сначала поощрять бредо-писателей, а потом целыми командами искать в этом логику.
23. ivanov660 4325 18.07.19 09:43 Сейчас в теме
(22)Самое разумное раздавать задачи в зависимости от компетенции. А за джунами обязательно должен быть присмотр.
26. skillman 5 21.07.19 17:31 Сейчас в теме
(22)
проще было бы не допускать таких к работе

Не согласен по поводу допуска, учить джунов как-то нужно, а код ревью дает еще рост квалификации
ivanov660; +1 Ответить
28. ivanov660 4325 21.07.19 19:05 Сейчас в теме
(26)конечно, джун же должен же как-то расти и притом что же он будет делать. Иначе это будет напоминать анекдот про молодого специалиста, которого не берут на работу из-за отсутствия опыта)
29. Бубузяка 62 15.12.19 10:21 Сейчас в теме
Автор, снимаю шляпу.
Руковожу группой разработки отраслевого решения (коробка на продажу) крупного франча. Команда разработчиков, включая меня, 4 человека.

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

Внутренних стандартов практически нет. В работе только Стандарты разработки от 1С, рекомендации с ИТС. Стилистику кода заимствуем из 1С:БП.

После того, как в истории хранилища появилась возможность выборочно сравнивать код объектов, мы отказались от маркировки изменений внутри "своих" объектов. Изменения объектов поставщика маркируем обезличенными маркерами, ибо, нефиг стране знать героев и тщеславие - грех :) Маркировка, конечно, нужна. Упрощает процесс обновления типовой части и гуманно по отношении к нашим коллегам на проектах и в группах сопровождения у потребителей. Некоторые обновляю типовую часть не дожидаясь нас.

Для проверки решения используем конфигурацию 1С "Автоматическая проверка конфигураций". Они ее регулярно обновляют, делая умнее. Сразу исправляем критические ошибки. После каждого релиза, это 3й знак в версии конфигурации по стандартам 1С, садимся за исправление того, что осталось, уменьшая технический долг. Ведь каждые 2 года сертификация на 1С-совместимо, а они там не разбираются где отступление от Стандартов, а где "проектное решение".
30. biimmap 1795 25.02.21 14:31 Сейчас в теме
Очередная правильная статья, направленная на истребление не грамотных людей, не соблюдающих стандарты.
Правила орфографии все понимают зачем нужны... Вот правила разработки также необходимы.

Правильный пункт про эскалацию между автором и проверяющим. "Авторам" надо внимательней прислушиваться к мнению более грамотных коллег. А то несут обычно такую чушь:
1. Оно и так работает.
2. Я считаю, что у меня нормальный код.
3. Я не понимаю зачем нужно переделывать.

Когда люди пишут под сертификацию 1С:Совместимо обычно не возникает вопросов зачем переделывать. Надо по умолчанию грамотный код писать, чтоб его и проверять то не требовалось.

Работал внутри 1С в отделе ЕРП. Заставляли переделывать даже код, которые многие сочтут соответствующий стандартам. С формулировками:
1. В отделе ЕРП такие подходы не используются
2. Код можно написать меньшим количеством строк
3. Код не достаточно эффективный.
user1564146; +1 Ответить
31. Serg O. 224 02.12.21 11:41 Сейчас в теме
для решения "эскалации конфликта" между Автором и Проверяющим...
наверное можно использовать ещё и различные метрики кода,
для "ОБЪЕКТИВНОСТИ" , например, используя внешние обработки такие как:

1) анализ повторений кода ( копиПастомер... см. https://infostart.ru/public/953497/)

2) анализ цикломатической сложности С ( https://infostart.ru/public/166182/ )

3) другие ошибки - стат-анализ кода - можно смотреть ошибки и рекомендации через конфигурацию АПК
или лучше через бесплатный Visual Studio Code (VSC)
для VSC - надо сохранить код в текстовый файл с расширением .bsl - и при открытии в VSС
в котором нужно 1 раз подключить расширение - Language 1C (BSL)
сразу "выявляются" неочевидные "ошибки" - длина строки не более 120, а длина функции не более 200 строк, обязательность описания функций и процедур, для Если - обязательно Иначе... и другие "полезные" и не очень ошибки
анализ когнитивной и цикломатической сложности VSC делает, на мой взгляд, лучше...
ограничение С < 15... очень отрезвляет, чтобы не писать запутанных функций с кучей Если и вложенными циклами

Есть и другие числовые метрики, но их трудно для 1С переложить...
Связность, стабильность/нестабильность модулей
Nср - среднее число строк в функциях и процедурах в 1 модуле = Длина кода / число F и P
Оставьте свое сообщение