Skip to content

Syn docs with ibm_db2 stubs #2316

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

Merged
merged 7 commits into from
Apr 6, 2023
Merged

Syn docs with ibm_db2 stubs #2316

merged 7 commits into from
Apr 6, 2023

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Feb 20, 2023

Supersedes #2300.

Generated using: php ../../php-src/build/gen_stub.php --replace-methodsynopses ../../ibm_db2/ibm_db2.stub.php ./reference/ibm_db2/

@Girgias
Copy link
Member Author

Girgias commented Feb 20, 2023

@NattyNarwhal could you review this as you seems to be the maintainer of the extension currently.

It would probably make a lot of sense to add various XML entities in language-snippets to prevent the needless repetitions for standard parameters across functions.

@@ -12,7 +12,7 @@
&reftitle.description;
<methodsynopsis>
<type>string</type><methodname>db2_stmt_error</methodname>
<methodparam choice="opt"><type>resource</type><parameter>stmt</parameter></methodparam>
<methodparam choice="opt"><type>resource</type><parameter>stmt</parameter><initializer>&null;</initializer></methodparam>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kocsismate seems like the gen_stub script doesn't take into account implicit nullability?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Girgias IIRC implicit nullability is not supported. I.e. you should get Parameter $varName has null default, but is not nullable exceptions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I didn't really get any issues from generating the method synopsis via gen_stub. But I suppose this is something that needs patching upstream then.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Girgias, do you have a suggestion about how to represent optional parameters in a stub that don't have a default value in the underlying implementation?
Currently, the stub is using null as default value for them (even when it is not true). Maybe there is a better way to represent that situation.

Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, I proposed this as a possible "convention".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NattyNarwhal
Copy link
Member

Discussion upstream about the autocommit stuff: php/pecl-database-ibm_db2#32

@NattyNarwhal
Copy link
Member

Discussion in the linked issue brought up gen-stub compatibility - will generated headers work on older PHP 8.x, even if it has newer constructs (for documentation and such) that appear in newer PHP? I know about the legacy arginfo header for 7.x, but I don't know what guarantees are present for 8.x. That'd affect what constructs we can put into the stub, and thus seemingly the documentation?

@kocsismate
Copy link
Member

@NattyNarwhal gen_stub.php available in PHP 8.2 takes version compatibility into account: php/php-src#8931

@phansys
Copy link
Contributor

phansys commented Feb 24, 2023

@Girgias, the stub ibm_db2.stub.php was updated.

@Girgias
Copy link
Member Author

Girgias commented Mar 9, 2023

@phansys just pulled the latest stub changes and regenerated the method synopsis. Does it look good to you now?

<methodparam><type>string</type><parameter>table-name</parameter></methodparam>
<methodparam><type class="union"><type>string</type><type>null</type></type><parameter>qualifier</parameter></methodparam>
<methodparam><type class="union"><type>string</type><type>null</type></type><parameter>schema</parameter></methodparam>
<methodparam><type>string</type><parameter>table_name</parameter></methodparam>
<methodparam><type>bool</type><parameter>unique</parameter></methodparam>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description for this parameter should also be updated, as it currently mentions integers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay done, the weird values is because it's initially some C int constants.

@Girgias
Copy link
Member Author

Girgias commented Mar 22, 2023

Hopefully this is now ready to be merged.

@Girgias Girgias merged commit 871b717 into php:master Apr 6, 2023
@Girgias Girgias deleted the ibmdb2-stubs branch April 6, 2023 13:45
@Girgias
Copy link
Member Author

Girgias commented Apr 6, 2023

ups forgot about this

claudepache pushed a commit to claudepache/php-doc-en that referenced this pull request Jun 1, 2023
* Sync with latest stub

* Update db2_statistics() unique param description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants