Rework DBAL compat layer #512
Conversation
ste93cry
left a comment
There was a problem hiding this comment.
I think that by making 2.13 the minimum required version (they recently added back support for PHP 7.2) we could probably cleanup some other code because they added a forward compatibility layer
| } | ||
| } | ||
|
|
||
| class MockDriver implements DriverInterface |
There was a problem hiding this comment.
It's not clear to me why you added this class instead of mocking the interface like I was doing before. What's the benefit of doing this or the blocker that makes it impossible now to do it?
There was a problem hiding this comment.
Psalm was complaining. But it didn't fix the issue anyway, so we could rollback this.
Would bumping DBAL requirement create issues?
There was a problem hiding this comment.
Would bumping DBAL requirement create issues?
I don't think so, but the best thing is to try. We couldn't bump the minimum version before simply because there was no support for PHP 7.2, which is now back
There was a problem hiding this comment.
I've investigated it further. The only additional FC is doctrine/dbal#4529, which is about ResultStatement, that we do not touch, so I don't see any advantages.
I'll try to push this PR forward, and on the code side it seems to work, but the static analysis is a complete mess.
There was a problem hiding this comment.
there was no support for PHP 7.2, which is now back
But isn't 7.2 dead?
There was a problem hiding this comment.
It's EOL, but we still support it mainly because Sentry must run also on legacy platforms, if possible
|
Closing in favor of #527 |
WIP, this includes #517 to check thoroughly for feasibility.
@ste93cry WDYT of this different approach?