Без лишних слов, перейду сразу к делу:
Компания, в которой я работаю, заказала услугу аудита кода стоимостью 300 000 рублей и сроком исполнения в календарный месяц. Целью аудита была «оценка качества кода» (именно такая формулировка присутствует в договоре), а предметом аудита были три проекта, образующие в совокупности туристический веб-портал с поиском туров и онлайн покупкой + ряд внутренних решений по автоматизации бизнес-процессов. Все три проекта реализованы на symfony2.
Исходя из суммарного объема переданного на аудит кода, стоимость услуг и сроки нам казались адекватными. По результатам аудита мы получили следующие два файла: отчет №1 и отчет №2.
Мы считаем результат аудита в виде этих двух файлов не соответствующим запрошенной стоимости и срокам исполнения. Для разрешения данного конфликта просим вас ознакомиться с содержимым отчетов и ответить на два простых вопроса ниже, а также написать свое мнение по поводу качества оказанной услуги аудита.
Комментарии (32)
kaljan
23.08.2018 11:49я думаю лучше нанять эксперта для проверки данной экспертизы
по факту согласен с 2morrowMan — похоже на работу линтераmultifinger Автор
23.08.2018 12:06Эксперта уже привлекали. Первая фраза стороннего эксперта, что это статический анализ переписанный своими словами. У меня первая мысль была аналогичная.
Quber
23.08.2018 19:29-1Я бы так не сказал. В аудите бОльшая часть написана именно человеком (не считая «служебные комментарии»). Это линтер бы не показал 100% (если только линтер не самописный). Однако есть вещи, определенные линтером и переписанные. Их не много, но они есть. И да за это тоже надо платить, потому что это тоже является частью аудита.
Собственно не понятны ваши ожидания. Если аудит получился не большим, видимо там больше ничего не нашли. Даже если бы там всё было хорошо и аудит бы получился ровно на одну страницу, это не значит что не нужно за него платить или платить меньше. Если аудитор провел полный объем работ по аудиту, вне зависимости от объема замечаний, вы должны заплатить ту сумму на которую договаривались изначально полностью.multifinger Автор
24.08.2018 06:41Ожидания таковы:
1) увидеть список критериев по которым была проверка
2) увидеть анализ структуры базы данных
3) увидеть что код читали включив мозг (например, замечание №2-3 из первого файла — объединить css-js файлы и скрин на кэш работы assetic — ну не туда надо смотреть ведь, а в код, в коде у нас есть в базовом шаблоне строки объединяющие и css и js файлы. если бы аудитор делал свою работу добросовестно, то заметил бы это. а так создается впечатление что впопыхах искали лишь бы что-то в отчет напихать.)
4) самое главное — увидеть выводы проделанного отчета, написанные понятным простому человеку языком, этого тоже нет. на словах нам заявили нечто вроде «ну это не говнокод, который мы обычно видим» и прочие жаргонные термины, а объективных итогов нет.
zartdinov
23.08.2018 12:30Недавно просили написать небольшую экспертную оценку для суда.
Качество аудита, сумма и сроки совпадают, клиент выступал в качестве ответчика.
Когда ознакомился с условием договора и планом аудита, понял, что клиент никак не участвовал в составлении этих документов. Вообще, мне кажется, не понимал для чего конкретно ему нужен этот аудит.
Но дело в итоге выиграли, потому что были еще нарушены сроки.
kether
23.08.2018 11:58
Имыш кродетсяаудиты пилятся.
Откровенная халтура. Отправьте им обратно файл с исправленной грамматикой и возьмите за работу 300К.
Angerslave
23.08.2018 12:02Привет, Макс! Аудит кода такой аудит кода. Сейчас тебе по скринам тут ещё что-нибудь напишут:)
Кстати, про лидеров что-то не понимаю. Компания, делавшая аудит — лидер рынка аудита?multifinger Автор
23.08.2018 12:05Да, сейчас будет минутка свободная, раскрою эту тему.
multifinger Автор
23.08.2018 13:53«Лидеры рынка» — это товарищи, делавшие аудит сами себя так называют, если быть точнее то вот такая подпись у них в письме идет:
1 место среди разработчиков в государственном секторе — РейтингРунета
1 место в CMS Magazine
1 место среди разработчиков Bitrix
1 место среди разработчиков Drupal
Лидеры в области PHP, JAVA
Таглайн — «открытие года»
Битрикс — лидер продаж Москва
werklop
23.08.2018 16:10+11) На кой черт вы заказали аудит? У вас что, одна школота работает?
2) Судя по грамматическим ошибкам, то деньги были оплачены зря, такую работу аудитора принимать нельзя и оплачивать тоже, это должно быть прописано отдельным пунктом договора, иначе вы сами лохи. Другое дело, если у вас у самих в школе по русскому языку было не больше тройки, тогда пофиг
3) Судя по отчету аудита, у вас реально работают школьники, взять хотя бы то, что у вас css/js-скрипты не минифицируются, дальше смотреть бессмысленно
movl
23.08.2018 18:49Аудит качества кода
Мнение сообщества о результатах аудита
Экспертная оценка результата аудитаЯ, конечно, не обладаю полным контекстом и не хочу никого обидеть, но со стороны все это выглядит достаточно смешно.
Quber
23.08.2018 19:10Просмотрел треть аудита. Собственно там большой объем. В целом могу отметить аудит не плохим. И даже хорошим. Единственное можно выкинуть всё то, что касается «служебных комментариев», это где то 30% вроде. Это не является проблемой абсолютно. По крайней мере в вашем случае. Большинство остальных комментариев действительно хорошие. Я знаю фреймворк симфони 2 как свои 5 пальцев. И ребята там почти всё правильно вам написали. Не понимаю ваше негодование на этот счет.
Да и еще им по хорошему надо добавлять ссылку, почему «рекомендуется» то что они пишут. А то рекомендуется кем не понятно, васей из соседнего подъезда? Но опять же рекомендации там хорошие и правильные, просто ссылок не хватает.
Приведённый аудит считаю адекватный по отношению к стоимости и достаточно профессиональным. Ко дням привязывать сумму нет смысла. Аудитор может сделать его и за неделю. Это его проблема. Может он день и ночь спать не будет.immaculate
24.08.2018 05:39Там большой объем идиотских скриншотов, которые должны были бы быть по идее в виде текстовых фрагментов кода, а между скриншотами — несколько предложений, являющихся корявым переводом вывода какого-то линтера.
Никакого анализа в данном аудите нет, такой аудит я могу набросать для любого проекта на любом ЯП за пару часов.
Автор, подскажите адрес и название компании, пожалуйста. Я хочу работать там, где за пару часов халтуры можно получить 300,000 руб. Точнее, меня совесть бы задушила делать настолько халтурный отчет, я бы все-таки рабочий день посидел и хотя бы минимально проанализировал и отфильтровал вывод линтера.
Quber
24.08.2018 07:52Если вы не голословничаете, объясните нам всем каким это интересно линтером можно было проверить скрин 10 в отчете 1??? Предвижу ваш ответ в духе «не знаю», «зайдите в гугл», «я вообще мимо проходил». Конкретный инструмент назовите нам
immaculate
24.08.2018 07:55Я не работаю с PHP. Возможно да, просмотрели исходники вручную, возможно использовали команду
ack-grep --php select.
Несмотря на это, обратите внимание, что даже здесь не обнаружили и не задокументировали явный копи-пейстинг кода.
Quber
24.08.2018 08:00Понятно, что к любому аудиту можно придраться. Точно также как и придраться к любому коду. Всегда будет, что сказать. Однако это не отменяет того факта, что аудит может быть смешанный как с привлечением автоматизированных средств так и ручных. Это абсолютно нормально. В этом аудите как раз такое и прослеживается. Достаточно много вещей описано именно после ручной проверки. Нет такого линтера, который бы показал, например, скрин 10 в отчете 1. Хотя аудит конечно можно было бы составить и лучше. Опять же мы возвращаемся к тому, что к любому аудиту ровно также как и к любому коду всегда можно придраться.
immaculate
24.08.2018 08:05Понятно, что ничего идеального нет. Идеальный код в жизни встречается исчезающе редко, идеальный аудит… Я в этой теме вообще не разбираюсь, если честно.
Но вот даже на данных скринах, например, сразу увидел копи-пейст. Если речь идет о качестве кода, копи-пейст всегда является более страшной вещью, чем один ручной запрос в базу. Потому что копи-пейст всегда будет головной болью тех, кому придется поддерживать проект. Копи-пейст экономит 5 минут при разработке, съедая просто неограниченное количество времени при поддержке.
Ну и просто сужу, что если бы мне кто-то поручил оценить данный код, я бы сделал гораздо более качественную работу и за гораздо меньшую сумму. Конечно, если бы кто-то предложил 300,000 рублей, я бы не отказался. Но все равно, уверен, что мой отчет был бы гораздо более информативным, подробным и качественным.
Из данного аудита по-моему невозможно сделать никаких выводов ни о чем. Ни о качестве кода, ни о каких-либо других метриках.
Quber
24.08.2018 08:09Всё относительно. Кто-то даже общаться не будет за сумму ниже 100 000. А кто-то за обеды в школьной столовой готов работать.
multifinger Автор
24.08.2018 08:21Из данного аудита по-моему невозможно сделать никаких выводов ни о чем. Ни о качестве кода, ни о каких-либо других метриках.
Вот именно! Разве может быть аудит без выводов?
multifinger Автор
24.08.2018 08:19Ручная проверка подразумевает включение мозга в работу анализа кода, а не только «вижу чистый sql — пишу что это плохо». В скрине 10 указано наличие прямых запросов в базу в классе ImportCommand, а в файле README в корне проекта, сказано, что данная команда используется для одноразового импорта данных из старой базы для развертывания проекта с нуля (т.е. это одноразовый код).
Данный код писался с требованием минимального простоя боевого сервера в момент обновления, решение использовать чистый SQL тут логично, под него не надо писать Doctrine-entity (пустая трата времени) + чистый SQL позволяет делать INSERT записей с сохранением ID, в Doctrine же для этого нужно менять описание на генерируемый вручную ключ + чистый SQL позволяет использовать особенности подсистемы хранения базы данных для ускорения вставки + не съедает память. Рекомендация переписать все это на Doctrine-сущности с учетом вышесказанного является неуместной. В итоге получается что обоснованно уместный SQL в коде импорта становиться ошибкой, если верить отчету. Соответственно, такому отчету верить нельзя. Это не придирка, такие заявления в отчете недопустимы, тем более без каких-либо объяснений.Quber
24.08.2018 08:23Если всё как вы описали, то полностью согласен с этим. Одноразовый код можно было написать в том формате как сделали вы. Вынесение SQL на отдельный класс тут излишне.
olku
23.08.2018 21:55Как сертифицированного аудитора, меня всегда интересовало что именно движет конкретной компанией, когда она заказывает внешний аудит. Не поделитесь? Можно в личку.
Аудит — это разовое мероприятие, которое оценивает соответствие предмета критериям. Аудит — прямые расходы, на которые компания идет вынужденно из-за требований закона (напр, GDPR) или регуляторов (напр, PCI). Эти двое задают чеклист и наказания. Лучшие практики, рекомендации и разного рода политики — добровольны и внутреннее дело компании. Внешним аудитом на них не заморачиваются, используют своих работников. За качеством на постоянной основе следят QA и лиды. На соответствие сайта законам может проверить обычный юрист, ему код не нужен. Денежные системы доверяют пентестерам, им кода все равно мало, ибо attack surface значительно шире. Хороший аудиторский отчет помимо находок (Findings) и рекомендаций, должен содержать цели аудита, примененные методы, критерии оценки и рамки (Scope), в которых аудит проводился. Ничего этого не заметил.Quber
24.08.2018 04:57Иногда аудит заказывают инвесторы, чтобы проверить команду которую они финансируют.
multifinger Автор
24.08.2018 06:26В конкретном случае заказчик — директор компании, хотел проверить меня — программиста, как я выполнил свою работу. Также была второстепенная цель проверить компанию аудитора с целью будущего привлечения к аутсорсу, т.к. была от знакомых на них положительная рекомендация. В заключении договора я, к сожалению, не участвовал, просто был перед фактом поставлен, мол «знакомьтесь»…
olku — я хочу по этому прецеденту написать и опубликовать небольшой мануал о том, что такое аудит и с чем его едят. Т.к. начав разбираться на тему «что же должно быть в аудите по хорошему» не нашел особо полезной информации в интернете — компании которые делают аудит на своих сайтах скромны в описаниях, ценообразование также не понятно.
Предлагаю вам, а также всем кто этой сферой интересуется, составить совместными усилиями документ, примерно такого содержания:
Типы аудита:
- такой-то
- такой-то
- такой-то
И по каждому пункту что в него входит и для чего делается. Возможно пару слов про методики и трудозатраты. Пишите мне в личку, как только у нас закончатся препирательства с аудитором, я буду готов потратить время на структурирование и публикацию полезной информацииQuber
24.08.2018 07:54Ну в первую очередь, если аудитором дается какая то рекомендация, то она должна быть чем то подкреплена и как то обоснована. В аудите, который вы привели есть обоснование всего у нескольких пунктов, но конкретных ссылок нет.
olku
24.08.2018 09:45Спасибо за подробный ответ, он многое проясняет. Например то, что критерии оценки качества кода у Вас и аудитора могут отличаться. У директора есть документы, где (Вами?) описаны критерии качества, которых обязаны придерживаться работники конкретной компании — Политики, Гайдлайны, Лучшие практики, Стандарты? Тот же PSR имеет рекомендательный характер и несколько уровней, а особенности фреймворка вообще не позволяют контролировать некоторые вещи, как того требуют анализаторы чистого ООП кода. Покрытие кода тестами, например, постоянный холивар — сколько достаточно именно этому бизнесу и почему? Правильных ответов больше чем один, а автоматические тесты проверяют коня в вакууме.
По аудиту инфосистем информации достаточно. Та же ISACA держит огромную цифровую библиотеку и тренирует аудиторов методикам COBIT и проводит вебинары и конференции. Не сочтите за рекламу. Хоть я и CISA лет 6 как, но ISO27K все равно нравится больше.
pi314
24.08.2018 06:12В описанной истории великолепна буквально каждая, отдельно взятая деталь — от договора, после выполнения которого невозможно сказать, был ли он выполнен или нет, и до отдельных оборотов речи в «отчетах», для автора которых русский, судя по всему, не родной! Точно, как в анекдоте: «Петька, приборы? 25! Что, 25? А что, приборы?»
Нет, произошедшее странное действо вообще не является «оценкой качества кода». Это можно бы было назвать выявлением очевидных косяков, допущенных разработчиками по причине некомпетентности, зачем-то сдобренным пересказом результатов статического анализатора своими словами… короче, тем, что обязаны были, но не сделали сами разработчики. Да, пару дней на написание такого «отчета» конечно же нужно — одних картинок вон сколько! А вот стоит ли это все заявленных денег, решить может только сам заказчик. Как минимум, косяки теперь можно (и нужно!) будет устранить. А в идеале, конечно, использовать страшную цифру как аргумент, что пора наконец нанять кого-то, кто знает, как разрабатывать проекты, как контролировать качество кода и что это, вообще, такое. Это может быть в сумме гораздо дешевле, чем в следующий раз снова вылавливать косяки на боевом сервере силами сторонних «экспертов» за бешенные деньги. Считайте, что просто пришлось доплатить деньги, сэкономленные на разработке (т.к. не допускать/отлавливать подобное, вообще-то, задача разработчика, а не «эксперта-аудитора»), и забудьте, каким странным словом это называлось в договоре ))
Оценка качества кода, это, вообще-то, совсем другое)) Это анализ кода (и архитектуры!) проекта с помощью разных объективных метрик, по внятно сформулированной методике, соответствующей конкретной цели, с которой проводится эта оценка. Целью может быть все, что угодно, от текущей оценки рисков или накопленного технического долга и до конкретного решения, покупать проект, или же нам пытаются впарить дохлую лошадь. И, да, настоящие эксперты в настоящих отчетах описывают методику, обосновывают ее выбор, полученные выводы и заключения, и дают конкретные рекомендации.
2morrowMan
По-моему, 80% этого отчета можно сгенерировать автоматически…
multifinger Автор
Мы думаем что так и сделали собственно