Update ext/oci8 methodsynopses based on stubs by kocsismate · Pull Request #601 · php/doc-en

kocsismate · 2021-05-13T22:13:48Z

No description provided.

Girgias

Looks mostly good to me other than a couple of changes, but I haven't really followed all the stub changes discussions.

kocsismate · 2021-07-18T14:30:00Z

@Girgias Can you please take another look?

cmb69

From a cursory look, this seems to be okay. I don't have any experience with OCI, and I don't have the time to check PHP 7 implementation regarding potential changes. Maybe @cjbj wants to have look?

cjbj · 2021-08-20T07:13:46Z

Gotta love the PHP community for being active on this! Thanks all.

Visually it seemed OK but my long-working build command php ../doc-base/configure.php && phd -o mydocs -f xhtml -P PHP -p book.oci8 -d ../doc-base/.manual.xml failed with

ERROR (/Users/cjones/p/phpdoc/doc-base/manual.xml:487:0)
End:
^
 XPointer evaluation failed: #xmlns(db=http://docbook.org/ns/docbook)
 xpointer(id('ingres.configuration.list')/*)

ERROR (/Users/cjones/p/phpdoc/doc-base/manual.xml:unknown)
 XPointer evaluation failed: #xmlns(db=http://docbook.org/ns/docbook)
 xpointer(id('mongo.configuration.list')/*)
failed.

The document didn't validate, here are the errors I got:
(If this isn't enough information, try again with --enable-xml-details)
 IDREF attribute linkend references an unknown ID "book.filepro"
 IDREF attribute linkend references an unknown ID "book.filepro"
 IDREF attribute linkend references an unknown ID "book.filepro"
[... and a bunch more]

Hints anyone?

cmb69 · 2021-08-20T07:53:18Z

@cjbj, I think you need to rebase this PR onto HEAD of master (also update doc-base and phd if not already done).

cjbj

@cjbj, I think you need to rebase this PR onto HEAD of master (also update doc-base and phd if not already done).

Good call.

cjbj · 2021-08-25T07:48:55Z

  &reftitle.description;
  <methodsynopsis role="oop">
-   <type>bool</type><methodname>OCICollection::assign</methodname>
+   <modifier>public</modifier> <type>bool</type><methodname>OCICollection::assign</methodname>


It seems OCICollection::assignElem wasn't updated.

Ohh, I think that must be because of the different capitalization (assignElem vs assignelem). :/

I created php/php-src#7405 to fix the inconsistency

That new PR doesn't seem to add the public modifier to OCICollection::assignElem ?

No, the new PR is unrelated from <modifier>public</modifier>. It is only added because the script always generates the access modifier for methods, while OCICollection::assign() didn't have one previously.

Yay for tidy-ups! Will you update this (601) PR with <modifier> for OCICollection::assignElem, Lob::truncate, getBuffering and setBuffering, and writeTemporary?

Yes, I've just pushed all the changes. In the meanwhile, I found out why OCILob::truncate() wasn't autogenerated: c3978de#diff-1ef7c5160d41789c5faf14de56219ac73b3039795ebbac6da6ebaa716cdd8d21L12 :D It's fixed now.

cjbj · 2021-08-25T07:51:31Z

  <para>
   <simplelist>
-    <member><xref linkend="oci-lob.truncate" /></member> 
+    <member><xref linkend="oci-lob.truncate"/></member> 


It looks like the prototype for Lob::truncate itself hasn't been updated.

What do you miss from Lob::truncate()? It looks good for me.

cjbj · 2021-08-25T07:52:17Z

-    <member><xref linkend="oci-lob.getbuffering" /></member>
-    <member><xref linkend="oci-lob.setbuffering" /></member>
+    <member><xref linkend="oci-lob.getbuffering"/></member>
+    <member><xref linkend="oci-lob.setbuffering"/></member>


The APIs for getBuffering and setBuffering don't seem to have been updated.

cjbj · 2021-08-25T07:52:48Z

  <para>
   Closes descriptor of LOB or FILE. This function should be used only with
-   <xref linkend="oci-lob.writetemporary" />.
+   <xref linkend="oci-lob.writetemporary"/>.


The API for writeTemporary doesn't seem to have been updated.

cjbj · 2021-08-25T08:02:02Z

-   <methodparam choice="opt"><type>int</type><parameter>flags</parameter><initializer><constant>OCI_FETCHSTATEMENT_BY_COLUMN</constant> + <constant>OCI_ASSOC</constant></initializer></methodparam>
+   <methodparam choice="opt"><type>int</type><parameter>offset</parameter><initializer>0</initializer></methodparam>
+   <methodparam choice="opt"><type>int</type><parameter>limit</parameter><initializer>-1</initializer></methodparam>
+   <methodparam choice="opt"><type>int</type><parameter>flags</parameter><initializer>0</initializer></methodparam>


I don't think showing the default as 0 is helpful to readers

Nice catch! I'll fix it in php-src as well

Submitted php/php-src#7429

kocsismate · 2021-09-14T07:39:38Z

Can I get a (hopefully) final review?

Girgias

Other than the minor nit in how the changelog is written (which well we're already inconsistent about so not that important), LGTM but then I don't know anything about OCI8

kocsismate · 2021-09-22T20:56:45Z

I rebased + added a new commit (964cbcd) which fixes a few minor things (+ an overloaded signature).

I'd love if I could merge this PR soon, after 4 months. :/

cjbj · 2021-09-23T08:04:57Z

@cmb69 @Girgias are you OK with this? Can it be merged?

kocsismate · 2021-09-23T12:57:15Z

Thank you, @Girgias !

cjbj · 2021-09-27T04:39:48Z

Thanks everyone!

kocsismate added the method synopsis label May 13, 2021

Girgias requested changes May 15, 2021

View reviewed changes

kocsismate requested a review from Girgias June 18, 2021 13:50

afilina added the Waiting on Review label Aug 17, 2021

afilina requested a review from cmb69 August 17, 2021 13:47

cmb69 reviewed Aug 17, 2021

View reviewed changes

cjbj requested changes Aug 25, 2021

View reviewed changes

Girgias requested changes Sep 14, 2021

View reviewed changes

kocsismate added 6 commits September 22, 2021 22:53

Update ext/oci8 methodsynopses based on stubs

ba2fb77

Address review comments

756c498

Add changelog entries for nullable parameters

5bf2bdd

Fix a few issues

610f3a7

Improve wording of changelog items

1cc65c4

A few description fixes

964cbcd

kocsismate force-pushed the oci8-stubs branch from fe73de6 to 964cbcd Compare September 22, 2021 20:54

Girgias approved these changes Sep 23, 2021

View reviewed changes

Girgias added 2 commits September 23, 2021 13:51

Use XML entities

b1a082a

Use XML entities

9b3a7bb

Girgias merged commit ed6de1a into php:master Sep 23, 2021

kocsismate deleted the oci8-stubs branch September 23, 2021 12:57

afilina removed the Waiting on Review label Dec 24, 2022

marcovtwout pushed a commit to marcovtwout/doc-en that referenced this pull request Dec 24, 2024

Update ext/oci8 methodsynopses based on stubs (php#601)

89e9f0d

Conversation

kocsismate commented May 13, 2021

Uh oh!

Girgias left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kocsismate commented Jul 18, 2021

Uh oh!

cmb69 left a comment

Choose a reason for hiding this comment

Uh oh!

cjbj commented Aug 20, 2021

Uh oh!

cmb69 commented Aug 20, 2021

Uh oh!

cjbj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kocsismate Aug 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kocsismate commented Sep 14, 2021

Uh oh!

Girgias left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kocsismate commented Sep 22, 2021

Uh oh!

cjbj commented Sep 23, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kocsismate Aug 30, 2021 •

edited

Loading