Ents Jun 28 2016 at 03:57

Внедрение code style в разработку

3 min

13K

PHP*

+12

Comments 31

heilage Jun 28 2016 at 06:27

1) А чем пре-коммит хуки не понравились? Или у вас в проекте настолько легаси, что файлы по 4000+ строк?
2) Не задумывались над тем, чтобы прогнать автоматическое переформатирование кода на всем проекте? Один раз и навсегда.

Ents Jun 28 2016 at 08:09

1) Пользоваться не так удобно как с web интерфейсе. Хук можно отключить и забыть об этом. В качестве быстрой проверки используются стандартный проверщик phpstorm (там тоже phpcs под капотом)
2) Задумывались, но отказались, так как ка хотели терять авторов последних строк

LionAlex Jun 28 2016 at 09:19

1) Проблемы теже, что и с pre-recieve, да еще и у всех настраивать надо
2) А кто потом это вычитывать будет и смотреть, что ничего не сломалось?

flr Jun 28 2016 at 11:47

А какие проблемы есть с pre-receive?

Ents Jun 28 2016 at 11:50

Во-первых — удобство (читать diff с подсветкой удобнее, чем просто список строк с ошибками в консоли)
Во-вторых — рубится на корню всякая возможность проигнорировать предупреждения проверщика стилей (а это бывает необходимо, когда исправление стилей требует исправления сотен классов, скажем, для переименовывания метода, который много где переопределяется)

А в данном случае все ошибки на этапе code review видны всем разработчикам, и они прям в комментариях могут договориться что какая-то ошибка не может быть исправлена и проигнорировать её

heilage Jun 28 2016 at 11:50

1) Кажется, что в среднем пре-пуш (или интеграция проверок на ide) дает более быструю обратную связь разработчику. Можно использовать в совокупности с пре-ресив хуками, конечно.
2) Автоматическое переформатирование должно гарантировать корректность кода. Если оно не может гарантировать корректность в каких-то специфических случаях — оно не должно трогать код и должно требовать от разработчика правильным образом его исправить.

Вот с частичной потерей истории в git — это да, существенный момент. Однако по себе могу сказать, не так уж оно и нужно — в один прекрасный момент поймал себя на мысли, что лезу в blame чисто посмотреть, кто такую херню написал :) Зачем, почему, как исправить — как правило понятно из кода (а если непонятно, то есть комментарии). Так или иначе, в пушистом легаси коде отсутствие комментариев и неявность намерений исходного автора может действительно быть существенной причиной не делать авто-преобразований.

Ents Jun 28 2016 at 13:30

Для быстрой обратной связи существует phpstorm, где прямо в момент написания кода подсвечиваются ошибки

UFO just landed and posted this here

heilage Jun 29 2016 at 06:06

Они работают там, откуда вы коммитите код. Ничто не мешает коммитить с личной разработческой виртуалки, где есть всё, что нужно.

UFO just landed and posted this here

Ents Jun 29 2016 at 10:01

главная ценность — простота и удобство пользования
Поддержка 15 виртуалок разработчиками (по одной виртуалке на каждого разработчика) — это существенная дополнительная работа, которая исключается в случае наличия централизованного инструмента

xotey83 Jun 29 2016 at 13:40

Если у вас есть инструмент поддержики конфигурации а-ля puppet, то проблемы нет.

Sannis Jul 5 2016 at 22:28

Вы разрешите доступ к своему ноутбуку и виртуалке на нём админскому паппету? Думаю нет :-)

xotey83 Jul 8 2016 at 16:31

Не к своему ноутбуку, а к виртуалке, на которой подняты все сервисы для разработки: php-fpm, nginx, mysql (для экспериментов), xdebug, туда же приностися конфигурация приложения (мы используем подобие etcd), и прочее. Вот эта виртуалка и управляется папетом.
А ноутбук используется в качестве тонкого клиента: phpstorm, браузер и ssh-клиент.

UPD. Уточню: виртуалка находится не на ноутбуке, а на девелоперском сервере.

Borro Jun 28 2016 at 08:00

Еще можно добавить к валидации https://phpmd.org/ — помогает писать более чистый код. Правда я бы подкрутил его настройки, например CyclomaticComplexity, NPathComplexity.

Ents Jun 28 2016 at 08:10

Да, это есть в планах. Когда буду делать — оформлю в виде отдельной утилиты, по аналогии с phpcs-stash

maximkou Jun 28 2016 at 08:04

В readme.MD у вас опечатка — brach вместо branch.

И еще немного провоцирующий вопрос: а какой стандарт стиля установлен в самом инструменте? (просто по коду местами используются разные стили написания). Чтобы не быть голословным: везде отступы пробелами, а например здесь — табами.

Ents Jun 28 2016 at 08:14

спасибо, опечатку и ошибку в стиле исправил
Как такой стиль кода не задан, но я по привычке пишу в symfony code style http://symfony.com/doc/current/contributing/code/standards.html

maximkou Jun 28 2016 at 08:20

Исправьте уж везде тогда) А не только там где я указал)

Как такой стиль кода не задан

Это конечно сугубо мое личное мнение, но раз уж инструмент следит за код-стайлом, он сам должен не иметь вопросов по тому самому код-стайлу.

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

andrewnester Jun 28 2016 at 08:46

У нас похожая ситуация с проверкой кода. На каждый пулл риквест запускается задача на CI сервере на проверку стиля. Если есть нарушения, пулл риквест будет иметь статус failed и бот отпишет где и в каких местах проблемы.
Плюс написан свой линтер, который у разработчиков вызывается локально на pre-commit hook
Всё круто работает, иногда правда с легаси кодом проблемы бывают

Ents Jun 28 2016 at 09:11

А проверяется только измененный код в рамках данной задачи, или весь проект целиком?

andrewnester Jun 28 2016 at 13:47

только изменённый, на CI есть задачи проверки и всего кода

LionAlex Jun 28 2016 at 09:22

Есть идея сделать проверяльщик интерактивным: чтобы можно было ответить на комментарий робота «исправь», и робот бы сам исправил ошибку

А не боитесь, что он там вам накоммитит не того?

Какой-то странный у вас кодстайл :)
https://github.com/WhoTrades/phpcs-stash/blob/master/lib/Checker/PhpCs.php#L11

class PhpCs implements CheckerInterface
{
	 /**
     * @var \PHP_CodeSniffer
     */
	private $phpcs;
	/**
     * @var Logger
     */
	private $log;
    /**
	 * @param Logger   $log
     * @param array $phpcs - ['encoding' => '....', 'standard' => '...']
     */
	public function __construct(Logger $log, array $config)
	{

Ой

var_export($core->runSync('refs/heads/feature/WTA-623', 'WT', 'sparta'));

Ents Jun 28 2016 at 11:28

Да, часть кода писалась в nodepad++, а там ставится таб, а не пробел. Потому вперемешку. Исправил

По поводу var_export() — app.php это пример использования библиотеке, для самой простой интеграции — webhook. У нас в живом проекте phpcs-stash подключается с помощью composer и работает внутри очереди

minamoto Jul 4 2016 at 14:07

Там есть опция специальная, позволяющая сразу при написании заменить таб необходимым количеством пробелов. Очень удобно, рекомендую.

LionAlex Jun 28 2016 at 09:38

Зачем дергать напрямую $GLOBALS['PHP_CODESNIFFER_CONFIG_DATA'], если есть PHP_CodeSniffer::setConfigData( string $key, string|null $value, [boolean $temp = false])?

У вас в код-стайле нет ограничения длинны метода? Кажется, RequestProcessor::processRequest излишне жирный и слишком много на себя берет.

А как-же юнит-тесты? :)

А вообще, спасибо, инструмент интересный и полезный в вашем случае. К счастью, у нас можно пофашиствовать и использовать хуки.

LionAlex Jun 28 2016 at 09:48

Ну и в дополнение – в настройках PHPStorm можно указать путь до рулсета phpcs и он будет подсвечивать ошибки стиля еще на этапе написания кода.

Ents Jun 28 2016 at 11:25

Мы это используем, но опять таки, phpstorm не гарантирует отсутствия ошибок в стилях (писал в комментах выше)

TheRat Jun 28 2016 at 11:29

Рекомендую посмотреть github.com/phpro/grumphp

Exileum Jun 28 2016 at 17:38

https://styleci.io/ — вот такое еще есть.

Sannis Jul 5 2016 at 22:30

Попробуйте phpcf — он умеет как находить, так и исправлять ошибки форматирования. Кроме того, если его запускать из git репозитория, он может проверять только те строки, которые были изменены с последнего коммита.

Show the best of all time