-
Notifications
You must be signed in to change notification settings - Fork 48
Add Exec and Query methods to FakePDO #6
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
Conversation
src/FakePdo.php
Outdated
} | ||
|
||
$sth = $this->prepare($statement); | ||
if ($sth->execute()){ |
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.
I believe this is the correct way to replicate the internal logic
src/FakePdo.php
Outdated
{ | ||
$sth = $this->prepare($statement); | ||
$sth->execute(); | ||
return $sth; |
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.
Not certain if anything new is needed here to replicate internal logic but this does run properly
So the fail on the build is coming from the
Whereas the tests run on PHP 8 which requires this to be typed as
But if we do that it will break on anything <PHP8. I wonder if this requires the tests to be run on the min supported version (PHP 7.1 according to composer.json)? |
Thanks! Please merge/rebase master! I've introduced separate |
So the last Psalm build on actions failed due to a Docker error... to make it build again I pushed some new But on the plus side, all checks now pass with the split out PHP7/8 classes! I'll update the Codeception branch at Codeception/module-db#20 to use a specific class too. |
Thanks! |
Adds other public methods (rather than just
prepare()
toFakePdo
)The current
master
branch state of Psalm surfaces the following fails:This branch does not resolve these, but also does not add new fails.