Pull to refresh

Comments 111

Делается очень просто на git hooks. На любом языке можете написать хук.
Ну, StyleCop — он и в Африке StyleCop. Как и когда будут проверяться его правила — второй вопрос.
Насколько я знаю, CruiseControl очень хорошо справляется с такой задачей для Git'а.
Спасибо большое, посмотрим. Выглядит очень интересно, радует поддержка Xcode
UFO just landed and posted this here
Напишите, пожалуйста, если накурите что-нибудь.
StyleCop курить бессмысленно, он C# only. Есть более-менее свежий обзор проверялок стиля кодирования для C++, посмотрите его. В принципе, любое из упомянутых там средств легко подвесить к почти любой системе контроля версий на pre-commit hook.
Из блока «Are you done yet?» есть еще один выход: «Нет, но наступил дедлайн». И вот он (при условии, что выполнялся блок «Code well») ближе всего к «Good code».
Нет, стрелка из «Нет, но наступил дедлайн» ведет к пункту «Code Fast»
Лучше сделать хоть что-нибудь что можно дать клиентам. А потом после всех замечаний и пожеланий переписать все заново\дописать.
Постоянно меняя концепцию\архитектуру можно заблудится.
Не самое лучшее решение. Но одно из самых распространенных. И как результат проект даже еще не в полной работе, а сидит целая куча программистов. Одна половина пытается допилить «что-нибуть для клиента» до удобоваримой работы, а вторая пытается что наваять новое чтоб выдержала нагрузку и не падала от пука.
Именно так и происходит.
Если заложена неправильная архитектура, скорее всего проект будут переписовать. Вероятно — другие люди.
А если вроде все правильно, но постоянно меняются требования то при масштабировании архитектуры можно тоже нехило прогореть.
В этом случае может спасти, например, летальное и окончательное техническое задание.
Я в шоке от того, сколько плюсов может набрать комментарий с банально последним выпуском xkcd
… как и множество других вещей.
Я в шоке от того, сколько взрослых людей дрочат на эти плюсики и минусики.
Мне как-то казалось что люди плюсуют то, что им нравится и то, что они хотели бы видеть и впредь. Это не так?
Просто более распространенная практика, плюс не все внимательно читают правила и соглашения.
А никто для Mercurial не знает подобных инструментов?
Практически _все_ подобные инструменты работают с практически всеми системами контроля версий — почти везде есть механизм хуков, который позволяет до/после/вместо коммита в репозитарий запустить какой-то скриптик, который выдаст ответ «можно или нет» коммитить. Соответственно, все эти системы автоматического энфорсмента работают вообще вне привязки к каким-либо определенным системам контроля версий — у них есть файлы, на этот список файлов они натравливаются, в stdout — результат, в exit status — код ошибки.
А при этом валидатор (в случае git, mercurial etc) должен быть установлен на всех рабочих станциях и одинаково сконфигурирован?
в гите есть серверные хуки, думаю в ртути тоже…
Зависит от вашей модели разработки. Как правило, у вас все-таки будет либо какой-то центральный сервер, который хранит некий основной бранч, из которого выпускаются production releases, или, при полностью распределенной модели, в репозитарии просто объявлен такой бранч, который у всех немножко разный, но все его таскают друг к другу по определенным правилам и, например, у тим-лида в его личной копии репозитария состояние этого бранча и является тем, откуда выпускается production.

И в том, и в другом случае, можно повесить и на той, и на другой стороне вызов проверяльщика — и при push, и при pull, и на клиентах, и на сервере.

Подробнее про хуки можно посмотреть здесь — про Git, кратко про Hg, подробно про Hg.
Интересно, а есть ли что-то подобное для Team Foundation Server?
Есть более простой аналог, например — tfsccpolicy.codeplex.com/ — проверяет наличие комментариев. Я так понимаю, что его можно допилить по вкусу :)
Есть верить этому сравнению, то в TFS есть pre-commit hooks, соответственно, можно туда любую подобную проверялку привесить.
В ТФС есть гораздо более вкусные инструменты. Можно даже при коммите проверять компилится код или нет
Это можно, но и StyleCop-правила никто не отменял.
В TFS такие механизмы из коробки. Причем двух видов, особенно интересный — gated checkin.
Предусмотрена ли там система исключений? У нас тоже есть гаидлаины для кода, но иногда бывает, что фрагмент кода настолько простой, что нет смысла комментировать, или, что стилизованный по гаидлаинам код становится менее читаемым.
Во-первых, это странные гайды, если «стилизированный по ним код становится менее читаемым».
Во-вторых, набор правил для проверки можно выбрать в StyleCop. Кроме того, StyleCop имеет механизм расширений, под который можно написать свои какие-угодно правила (хоть бы «не требовать комментариев для методов короче 10 строк»)
В-третьих, в коде пре-коммит хука стоит, можно написать что-то в духе «если текст коммит-сообщения содрежит превым символом »!" — не анализировать код на валидность). Может помочь, когда нужно в авральном режиме пофиксить баг, не задумываясь о всяких стандартах.
На счёт во-первых: простой пример — в гайдлайнах «свойства должны быть записаны кэмелкейс».

Есть орм, который отображает as-is поля базы на объекты. В базе, соответственно, поля через _. Типа foo_bar.

Что делать?
Во-первых, орм должен давать настраивать способ отображения полей базы в в поля объектов (многие орм это умеют).
Во-вторых, SVNStylecop можно настроить так, чтобы он не проверял автоматически-сгенерированный код (он даже по дефолту файлы, которые студия для форм генерит не проверяет). Делается это путем правки фильтра файлов, подлежащих проверке (см. в статье раздел правки конфигов).
Угу, примерно такого варианта я и ожидал.

Просто, не вдаваясь в подробности, орм (пхп) — это простейший активрекорд, без конфигурации и схем, просто превращающий массив с данными из базы в объект.
Извините, но, по большому счету, статью можно свести к фразе «в SVN можно в момент выполнения коммита вызвать что-то, что проверит новоприбывший код — я туда повесил SVNStyleCop и счастлив». Первое известно из документации, про второе — если уж пишется статья — хотелось бы хоть каких-то пояснений, сравнений с аналогами, разумного выбора альтернативы.

Собственно, в деле автоматического энфорсмента каких-то политик отнюдь не в этот несчастный вызов svn/git/hg-хука всё упирается. Дьявол в деталях, которые Вы, собственно, почти все пропустили. А именно:
  • Какие стандарты есть для используемых языков
  • Как (и нужно ли) их творчески доработать
  • Как убедить команду им следовать
  • Как правильно написать эти самые соглашения по написанию кода и до какой степени их можно энфорсить (крайняя степень — в принципе прогонять _всё_ через автоформатирование перед коммитом)
  • Как эти соглашения ложатся на язык описания стилей выбранного инструмента автопроверок
И так далее, и тому подобное. Понятно, что на 90% — это — религиозные вопросы, поэтому вытекает общий следующий вопрос — как найти наиболее разумный компромисс в команде и не разругаться по поводу религиозных вещей. Кто и когда должен уступать, кто не должен. Как выстроить взаимоотношения в команде для того, чтобы такие решения принимались легко и непринужденно… Вот это было бы очень интересно почитать…
Вы все верно пишете, однако создание стандартов по кодированию не есть тема этой статьи. По этому делу и вправду можно вести религиозные войны, по этой теме написаны сотни книг. Я в качестве примера привел соглашения комманды RSDN (которые, к стати, на 95% совпадают с моими), но ответственности за утверждения «программируйте вот так и только вот так!» я на себя брать не хочу. А статья — просто короткий «how-to» по инструменту, который на Хабре еще не освещался.
Эцсамое, а если svn-сервер живёт далеко и на дебиане, это чудо техники через моно завести можно?
StyleCop — это открытый продукт Microsoft под лицензией MS-PL. На Mono, насколько я знаю, не работает.

Под Mono есть собственный проект Gendarme, который похож на StyleCop, но исповедует немного другие принципы и мощности проверок. И он, кстати, под MIT-лицензией, а не под MS-PL.
Так gendarme на откомпиленую сборку натравливается и форматирование кода проверять не в состоянии.
Ну, я поэтому и говорю про «несколько другие принципы и мощности». Частично их функциональность пересекается, частично — нет.
Gendarme — это FxCop. Тоже полезно, впрочем.
Ну, не совсем, насколько я понимаю — в Gendarme меньше проверок на валидность кросс-библиотечных взаимодействий и внешних интерфейсов и чуть больше проверок на внутренности, но в целом — да — согласен, такое сравнение, пожалуй, более корректно.
Программисты-лентяи наконец-то прочитают внутренние соглашения компании по кодированию
Я бы сказал они выучат стандарт, когда исправят тысячу ошибок в 6ти тысячах строках кода. Это реально работает, привет PHP_CodeSniffer
Спорить с роботом — это все равно, что разговаривать с телевизором
Добавлю цитату в избранное.
Подскажи, как вы добавляется цитату в избранное, точнее в какое избранное?

P.S.
Спасибо за PHP_CodeSniffer.
На рабочем столе лежит файлик «quotes.txt» открываю его и с новой строки записываю цитату.
echo "New quote" >> ~/Desktop/quotes.txt
UFO just landed and posted this here
UFO just landed and posted this here
Но разве качество кода определяется оформлением?
Конечно нет. Но если взять непересекающиеся множества «хорошо оформленный код» и «плохо оформленный код», и пересекающееся с ними обоими множество «качественный код», то в «хорошо оформленном» будет больше «качественного», чем в «плохо оформленном». Т.е., я считаю, что хорошее оформление — это один из компонентов качественного кода. Конечно, не единственный и, может быть, не основной. Но с ним лучше чем без него.
абсолютно согласен — будут комитить говнокод, зато оформленый по всем конвеншенам.
Против говнокода есть Code Review, но после робота ревьюер по крайней мере будет нормально отформатированный код читать и не отвлекаться на формальности.
Ага, ага. Так раздражает на Code Review что самому говорить, что слушать замечания в духе «а тут пробела не хватает, надо исправить». Жалко времени своего и людей.
Я не имел ввиду, что это не надо использовать. Надо. Обязательно. Если еще прикрутить проверку проверку на максимальную длинну метода, на отсутсвие неиспользуемого кода итд… — будет еще лучше.
Просто название статьи «Как не дать программисту написать плохой код». Плохой код всеравно будут писать.
Правила на максимальную длину метода для StyleCop есть. На «отсутствие неиспользуемого кода» — не видел, но всегда можно написать самому (StyleCop поддреживает расширения). Но, конечно же, плохой код все-равно будут писать. Но может хоть чуточку меньше…
У нас это дело решается на уровне файла проекта, в который включена насильственная проверка StyleCop-ом. В результате неотформатированный код просто не скомпилится — как у девелопера, так и на билд сервере. Поскольку работаем не в детском саду, не припоминаю чтобы билд падал изза StyleCop-а (хотя как ругали StyleCop слышал неоднократно)))))
Еще никто не додумался локально подправить файл проекта, отключив проверку StyleCop? :)
Нет, но иногда создаются новые проекты без этой проверки. Тогда у девелоперов на этом проекте халява до того момента, как я запущу в него руки :)
А еще можно даже оставив проверку StyleCop тупо комитнуть некомпилящийся код.
Можно. Но это уже совсем другая история — и средства борьбы соответственно совсем другие. Я того и написал — не в детсаду работаем: люди, которые регулярно коммитят некомпилируемый код на проект не попадают.
Вещь полезная, но с названием статьи никак не вяжется.
Почему? Мне кажется в ней речь как раз о том как не дать программисту комитнуть в SVN код, не отвечающий некоторым формальным критериям. Слово «комитнуть» писать в заголовке не хотел — кто-то может не понять, да и слово какое-то не русское.
Соответствие кода «некоторым формальным критериям» не является признаком его «хорошести».
Хотя повторюсь — инструмент полезный.
Ну конечно, не является. Зато соответствие некоторым другим является однозначным признаком плохого кода. Если у вас в одном месте классы называются в стиле Паскаль, в другом — кеМел, если объявление интерфейсов и классов не откомментированы, если у вас в коде встречаются методы в 1000 строк длиной — это плохой код. И это можно проверить при коммите. И это можно не дать коммитнуть. Конечно, это не даст 100% результата (даже 50%-го не даст, я думаю) но иногда помочь может.

Да и не знаю я как можно было по-другому назвать статью.
То есть, если у меня будет правильно отформатированный/откомментированный код и методы по 30 строк — это будет хорошим кодом?
Надеюсь вы сами понимаете, что это не так.
Мы пришли к часто приводящим к непониманию определениям «необходимости» и «достаточности» выполнения условия для достижения результата. Я говорю, что хороший стиль оформления и комментарии есть «необходимое» условие хорошего кода. Вы говорите, что оно не есть «достаточное». С этим я тоже согласен, что и выразил в предыдущем комментарии.
Я не спорю с вами, и мы также используем нечто подобное в своей практике.
Я только указал, что ваше решение не решает заявленную в названии статьи цель.
Или решает, но частично. Не воспринимайте мои слова как критику — это всего лишь замечание.
А в заголовке и не написано «Как заставить писать хороший код».
И что?
Представленный инструмент не запретит мне написать плохой (хоть и хорошо оформленный) код. А именно это и заявлено в названии.
Для Java навскидку есть checkstyle и pmd. Вообще говоря, можно подключить достаточно мощный статический анализатор, который будет выявлять потенциальный проблемы. Но всё это увеличивает время коммита, так что в какой-то момент надо остановиться и навешивать все эти проверки на ночные сборки.
Кроме того, в Eclipse можно настроить Save Actions, которые приводят код в порядок: форматируют, организуют импорты, даже убирают лишние скобки.
>> Я (и еще 3 человека) попросили автора развить мысль и написать статью об этом,
>> но она так и не появилась. И я решил разобраться сам.

Стыдно. Ведь это был я :-/
Ну, я честно признал за Вами источник идеи и дал на Вас ссылку. Надеюсь, какой-никакой плюсик в карму Вам упадет.
В Eclipse есть схожая возможность — автоматически применять указанное форматирование к коду в момент сохранения файла. В этом случае программисту нет необходимости вообще задумываться о форматировании кода в соответствии с соглашениями. Отсутствие комментариев или неправильные имена опять же можно сделать ошибками компиляции вместо предупреждений.
Запрет на отправку кода в репозиторий — штука полезная, но лучше предупреждать программиста о таких ошибках сразу а не в момент коммита.
Так stylecop интегрируется с VS и есть плагин для Resharper, так что разработчик увидит о потенциальной ошибке сразу в момент написания кода, даже компилировать не надо.
Мне кажется, не ту задачу вы пытаетесь решить.
Нанять на работу программистов, которые пытаются «смухлевать» и надеются, что «вдруг никто не заметит», и пытаться с ними бороться — это ли не абсурд?

Ну и та самая проблема, о которой вы упомянули: программист всё ещё может закоммитить плохой код (классы, методы и переменные, названные по правилам именования, но бессмысленно, комментарии с непонятной белибердой и т.д.) — то есть заявленная цель «не дать программисту написать плохой код» не выполнена. Вы всего лишь заставите ФОРМАТИРОВАТЬ ПЛОХОЙ КОД, что является, по-моему, бессмысленной тратой ресурсов.

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

Кроме того, речь шла не только о форматировании, но и еще как минимум стиле именования сущностей и написания комментариев, что уже не мало.

А то, что это не главная задача, я согласен. Конечно, важнее самодисциплина и отбор людей для проекта. Но иногда нужно что-то делать с уже имеющимися людьми, а не ждать «адептов и гуру».
Почему сразу смухлевать? Все мы человеки, все мы можем ошибаться.

Да и проверять хорошо форматированный код на предмет является он плохим или хорошим быстрее чем плохо форматированный, не так ли? А значит экономия ресурсов скорее всего будет налицо — пускай 5 джуниоров потратят по 5 лишних минут на форматирование, чем один сеньор потратит по 3 лишних минуты на чтение неотформатированного.
Кстати, еще может быть полезен хук отслеживающий правильность укладывания файлов в репоз. А то у нас, например, структура регламентирована, а вот некоторым программерам на нее похер, могут наложить своего говнокода прямо в корень репоза или еще как. Но хуки таки реально спасают.
В моей предыдущей конторе была система code-review. Процесс коммита выглядел так:

1. Сначала программист «коммитил» код в ревью-систему
2. Затем уполномоченным программистам приходила нотификация и они смотрели код
3. Затем либо принимали, либо отвергали коммит (заводили «дефекты»)
4. Если код принимался, программист посылал этот код в репозиторий.

Послать можно было только заапрувленный код, там чексуммы изменений создавались и сравнивались хуками.
И как? Работало\помогало?
Да, но это требует очень сильной дисциплины. Сначала был период «притирки», возмущения всякие, споры, но потом привыкли.

Главный плюс от такого подхода в том, что возникает простой и малоконфликтный способ внесения замечаний. Нету бурных словесных перепалок, всё чётко и формализованно: дефекты, история дефектов, обсуждение.
Более эффективно, на мой взгляд, попробовать сократить рутинные операции по приведению кода к «валидному» состоянию, внедрив подобный механизм непосредственно в процесс кодирования, еще лучше — добавить автоматизацию.
В простейшем варианте становится существенно короче обратная связь, т.е. разработчик сразу получает информацию о том, что часть кода не соответствует стандарту.
В качестве другого варианта — более жесткие ограничения, не позволяющие вводить некорректные символы и редактировать некоторые части кода.

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

Вполне нормально с таким подходом выходило.
когда-то давно юзал подобное с свн. буквально через неделю или 2 мне это надоело. перед каждым комитом вылизывать код нудно.
сейчас мне кажеться что mercurial/git для этого лучше подойдут, потому что stylecop можно было бы (если можно) повесить только на push, но не на commit, и стилизовать код только перед отправкой в общий репозиторий. но настраивать мне лень и кажеться что будет то же самое — быстро надоест.
да и к тому же хорошо отформатированный код != код хороший.
Ну, стиль форматирования один раз настраивается в студии или решарпере и дальше применяется ко всему файлу одной комбинацией клавиш. Стиль именования сущностей в любом случае есть в памяти — тут просто работает проверка, не опечатался ли программист. А вот требование наличия комментариев — это да, это может раздражать. Но тут уж как политика партии решит — можно заставлять себя и окружающих это делать, а можно плюнуть.
А я за введение телесных наказаний в IT :)

Закоммитил говнокод — получил в пятак.
Сломал билд — высекли розгами.
А уж за просранный релиз… ууууу… Быстро научит адекватно оценивать время :)

Опять же у медали две стороны — за отвратительное управление проектом, команда может устроить менеджеру тёмную :)
Ещё можно идею развивать:
  • Боксёрский ринг для споров об архитектуре проекта.
  • Битвы отдел на отдел.


Для сильных программистов и ловких менеджеров :)
Согласен, но только вместе с телесными поощрениями.
— написал комментарии — получай массаж
— пофиксил некритический баг — поход в сауну
— пофиксил критический — сауна с девочками
1. рекомендуем плохого сотрудника, который регулярно пишет унылый код
2. фиксим за ним критические баги
3. получаем абонемент в сауну с девочками
4.…
5. профит
Как тут не вспомнить древний баян:
«Распоряжение начальника:
Каждому сотруднику в офисе предоставить пульт с двумя надписями:
— кофе
— минет
У хороших сотрудников — это кнопки, у плохих — лампочки.»
количество пробелов перед методом хоть как-то повысит качество кода?
Если сделает его более читаемым, то несомненно повысит.
я думаю программистов можно поделить на две категории:
— те которым мы доверяем (а они и так правильно пробелы расставляют)
— и те которым мы не доверяем (а за ними все равно нужно проверять код, и количество пробелов — совсем не самая страшная ошибка которую они могут сделать)
Нескажить — В правильно оформленном коде находить ошибки намного легче и быстрей.
Главное чтобы говнокодеры или не программисты не получили такой инструмент в свои руки, иначе они наустанавливают саботажных правил коммита.

Говорил с техдиром одной, сейчас уже закрывшейся фирмы, вершиной мастерства которого было написанием формул на VB для заполнения ячеек в экселе. Он мне, с пеной у рта, доказывал что конструкция вида if() ... endif по всем параметрам намного лучше чем if(){ ... }. Он всех своих программеров писавших на php(там возможны оба варианта) заставлял использовать её :)
Это уже дело привычки и внутреннего соглашения кодирования.
На мой взгляд, в этом случае, правильнее ориентироваться на несколько программистов каждый день работающих с этим, чем на привычку техдира, которому изредка нужно в коде что-то посмотреть.
Если Вам не нравится соглашения это еще не значит что они ложные/плохие то или терпите или меняйте работу. В самом начале работы в ИТ тоже удивлялся нахрена столько условностей и тонкостей документа оборота и всякой всячины которые тормозят процессы, но сейчас по многим пунктам на которые плевался сам веду мукалатуру и требую от других.
У себя на работе пользовался Changelogic (http://www.changelogic.com/)
Из плюсов могу отметить:
— Создание веток на каждое изменение (change)
— Проверка кода — с просмотров диффов и прочее.
— Интеграция с основной версией
— Версии
— Интеграция с eclipse

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

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

Но главное, что даже когда такой робот есть, лучшее, что он может гарантировать — это вот эти самые мелочи. Как только вы пытаетесь применить автоматику к более серьёзным требованиям (например, «каждая функция должна быть документирована»), он сразу начинает пробуксовывать.

Например, робот требует документирующего комментария к каждой функции. А у программиста аврал, и он не спал 36 часов. Что он напишет, чтобы робот успокоился? «Finds optimal path» в комментарии к функции FindOptimalPath. Или вообще «Does some stuff». И даже если комментарий написан добросовестно, как роботу проверить, не забывает ли программист обновлять его при изменении логики кода? Как обеспечить, чтобы у переменных, функций, классов были понятные имена, отражающие их сущность? Как обнаружить роботом спагетти-код и десятки других антипаттернов?

В общем, какая-то ценность у автоматических средств контроля есть. но не надо выдавать их за волшебный способ не дать программисту написать плохой код. Это может сделать только другой (хороший) программист.
Sign up to leave a comment.

Articles