-
Notifications
You must be signed in to change notification settings - Fork 800
Generate ext/pdo methodsynopses based on stubs #494
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
1dc3cfa
to
b061511
Compare
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.
This looks ok to me now. I still have a feeling we missed something. @cmb69 What's your opinion?
From a quick look, this looks okay. Of course, there is the issue that some of these methods may not return false on failure for ERRMODE_EXCEPTION, but I don't think it's worth to explicitly document that (IMHO, letting clients choose how issues are reported is a bad idea in the first place). |
@cmb69 Yeah, I noticed that too, but I think we can make a separate PR for that. The return values will have to be adjusted for the new signatures. |
<varlistentry xml:id="pdo.constants.fetch-default"> | ||
<term> | ||
<constant>PDO::FETCH_DEFAULT</constant> | ||
(<type>int</type>) | ||
</term> | ||
<listitem> | ||
<simpara> | ||
Specifies that the default fetch mode shall be used. | ||
</simpara> | ||
</listitem> | ||
</varlistentry> |
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.
Hello, just stumbled over this constant. As nice as it would be, it does not exist (8.0.6). Or is this planned?
Fatal error: Uncaught Error: Undefined constant PDO::FETCH_DEFAULT
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.
We documented it a bit prematurely... since the constant has just been added by php/php-src#6937, it's going to be available in the next patch release
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.
Please add that info ("Available as of PHP 8.0.7.")
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.
Submitted: #597 Although I'm wondering if this information should be added to the usages of the constant?
No description provided.