-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updating all dependencies, including omnipay/common to ^3.0 #104
base: master
Are you sure you want to change the base?
Conversation
composer.json
Outdated
"guzzle/common": "^3.9.2", | ||
"guzzle/http": "^3.9.2", | ||
"guzzle/parser": "^3.9.2", | ||
"guzzle/plugin-mock": "^3.9.2", | ||
"guzzle/stream": "^3.9.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These need to go, but for now they can stay. Gives us the ability to do "symfony/event-dispatcher": ">=2.1",
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Holding this up is https://github.com/vimeo/omnipay-vindicia/blob/update-dependencies/src/Message/AbstractRequest.php#L138 taking in a Guzzle/Http/ClientInterface, instead of the new vindicia/common ClientInterface which also causes this to fail since the signature doesn't match: https://github.com/vimeo/omnipay-vindicia/blob/update-dependencies/src/Message/AbstractRequest.php#L140
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue with https://github.com/vimeo/omnipay-vindicia/blob/update-dependencies/src/Message/AbstractHOARequest.php#L28, but just on the __construct level, parent is AbstractRequest so there's no signature clash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really we just need this to change: https://github.com/vimeo/omnipay-vindicia/blob/update-dependencies/src/Message/AbstractRequest.php#L145 along with taking an Omnipay\Common\Http\ClientInterface
instead of a Guzzle\Http\ClientInterface
and this should work out of the box.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/vimeo/omnipay-vindicia/blob/update-dependencies/tests/EventEmitterTest.php#L21 also needs to be updated
…estCase::setUp(): void` errors.
No description provided.