search

biplane / yandex-direct

PHP library for Yandex.Direct API.
MIT License
44 stars 23 forks source link

можно поднять версию symfony/event-dispatcher в зависимостях до 5.x ? #26

Closed rik43 closed 4 years ago

rik43 commented 4 years ago

конфликт при установке на laravel 8

rik43 commented 4 years ago

и symfony/options-resolver наверное тоже понадобится также до 5

rik43 commented 4 years ago

думал без изменений заработает, но нет.

https://github.com/rik43/yandex-direct/commit/b50808e29a73dcb0dad59c4a7dce88adf5e4e840

Event в другом неймспейсе. и у dispatch порядок аргументов обратный. в остальном работает.

yethee commented 4 years ago

К сожалению, без нарушения обратной совместимости не добавить поддержку symfony/event-dispatcher:^5.0. Нужно отказываться от поддержки версий ~2.7 и ~3.0. В ветке master начал масштабный рефакторинг, но пока не очень активно продвигается.

rik43 commented 4 years ago

почему не добавить? если изменения только в том что я выше написал, то для Event можно сделать обертки расширяющие оба эти Event класса и инклюдить один из них, который есть. для dispatch можно написать тоже метод - обертку, которая вызывает в одном из двух форматов смотря какой подключен.

хотя я не знаю как это повлияет на пользовательский код, так и не понял нужен ли реально event-manager пользователю sdk.

rik43 commented 4 years ago

еще раз перечитал readme. как я понял вся эта бодяга с event dispatcher нужна для логирования? может оно и не надо вообще? всмысле добавить вместо этого только возможность передать свой LoggerInterface и через verbose туда будет писаться или полный запрос или только основное.

имхо этот функционал нужен наверное только для разработки и дебага? или кому то это может быть надо в продакшене? непонимаю какие кейсы. просто без этой зависимости сразу много проблем бы ушло. то есть сделать её необязательной зависимостью и поддерживать только старый вариант по 4.x?

yethee commented 4 years ago

может оно и не надо вообще? то есть сделать её необязательной зависимостью и поддерживать только старый вариант по 4.x?

Да, рассматривается вариант, чтобы сделать эту зависимость опциональной.

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

rik43 commented 4 years ago

по моему LoggerInterface выглядит логично.

я своб потребность пока закрыл (форкнул и поправил, у меня все работает), но в перспективе хотелось бы вернуться. Удачи в общем!

yethee commented 4 years ago

PR #29 добавляет поддержку symfony5 для ветки 4.x. Все тесты проходят, но у меня, к сожалению, нет сейчас времени проверить в каком-нибудь проекте. Если есть желание проверить работоспособность, можно попробовать версию из PR:

composer require biplane/yandex-direct:dev-symfony-5
rik43 commented 4 years ago

спасибо, переключился на вашу версию. всё работает! но у меня в проекте пока все readonly. чтение кампаний/обьявлений/ключей + несколько отчетов. этот функционал работает.