Wrap pdo instances#3544
Wrap pdo instances#3544lcobucci wants to merge 4 commits intodoctrine:masterfrom lcobucci:wrap-pdo-instances
Conversation
|
|
||
| if (isset($params['pdo'])) { | ||
| $this->_conn = $params['pdo']; | ||
| $this->_conn = WrappedPDOConnection::fromInstance($params['pdo']); |
There was a problem hiding this comment.
It still would be a breaking change since it's no longer a PDO instance (won't match a type hint, can't call PDO-specific methods, etc).
There was a problem hiding this comment.
Not according to our docblocks...
There was a problem hiding this comment.
We never expect to handle raw PDO connection in the internals
There was a problem hiding this comment.
True. I didn't notice it was about the internal API. But still, even if we can afford introducing this class, I don't think we should. Having two PDO connections which share most of the code is not a proper solution. I'd like to propose #3549 instead.
There was a problem hiding this comment.
@morozov I'm fine with not adding this adapter 😄 I'd suggest to port the tests to your PR, just to ensure that the basic stuff works fine.
| use function assert; | ||
| use function func_get_args; | ||
|
|
||
| final class WrappedPDOConnection implements Connection, ServerInfoAwareConnection |
There was a problem hiding this comment.
I'm not really happy naming-wise and am open for suggestions.
PDOConnectionAdapter?
|
Closed as per #3544 (comment) |
Summary
This is an alternative of #3543 created by @alcaeus and @nicolas-grekas.
I'm not really happy naming-wise and am open for suggestions.