Skip to content

Conversation

@IMSoP
Copy link
Collaborator

@IMSoP IMSoP commented Apr 19, 2021

This constructor takes a large number of options, most of which
were effectively undocumented, leading to a very high volume of
User Notes. Some examples and caveats from these notes have been
incorporated.

The descriptions here are largely based on reverse-engineering
the source code (which isn't particularly well commented) and
experimenting locally, so may not be 100% accurate, or capture
differences between versions.

@IMSoP IMSoP force-pushed the soapclient-constructor-rewrite branch from 8d3e513 to 8d1db39 Compare April 19, 2021 20:49
@afilina afilina marked this pull request as draft August 6, 2021 14:35
@IMSoP IMSoP force-pushed the soapclient-constructor-rewrite branch from 28b193c to cf4092e Compare March 20, 2022 22:52
@IMSoP IMSoP force-pushed the soapclient-constructor-rewrite branch from cf4092e to 9845255 Compare April 4, 2022 21:34
This constructor takes a large number of options, most of which
were effectively undocumented, leading to a very high volume of
User Notes. Some examples and caveats from these notes have been
incorporated.

The descriptions here are largely based on reverse-engineering
the source code (which isn't particularly well commented) and
experimenting locally, so may not be 100% accurate, or capture
differences between versions.
@IMSoP IMSoP force-pushed the soapclient-constructor-rewrite branch from 8791d10 to 8a29bad Compare April 9, 2022 18:36
@IMSoP IMSoP changed the title [WIP] Rewrite SoapClient constructor documentation Rewrite SoapClient constructor documentation Apr 9, 2022
@IMSoP IMSoP marked this pull request as ready for review April 9, 2022 18:41
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! I very welcome improvement.

@afilina
Copy link
Contributor

afilina commented Dec 22, 2022

@IMSoP Pinging author

@IMSoP
Copy link
Collaborator Author

IMSoP commented Dec 22, 2022

It looks like @cmb69 merged this manually at the time, despite the comments: 6105da1

I'll leave this PR open as a reminder, and if I get time, will raise a new PR with the suggested improvements.

@cmb69
Copy link
Member

cmb69 commented Dec 22, 2022

It looks like @cmb69 merged this manually at the time, despite the comments: 6105da1

Ugh, that might have been by accident. Anyway, since it has been committed and likely already been translated, leaving as is might be the best option.

@afilina
Copy link
Contributor

afilina commented Aug 2, 2023

I'll leave this PR open as a reminder, and if I get time, will raise a new PR with the suggested improvements.

@IMSoP Might you have the time to raise the new PR?

@afilina
Copy link
Contributor

afilina commented Oct 13, 2025

@IMSoP There has been no update in several years. Is there still an interest in completing this PR? There are also some merge conflicts to resolve.

@IMSoP
Copy link
Collaborator Author

IMSoP commented Oct 13, 2025

@afilina Basically everything in this PR will be a merge conflict, because it was accidentally merged offline. So everything in this diff is actually live in the PHP manual, and has been for several years.

In the meantime, as you've probably guessed, I'd completely forgotten about this. Having changed PCs, I don't currently have a dev environment set up to test some of the riskier markup changes, and life keeps happening, so I doubt I'll get back to it any time soon.

Effectively, this is now more of a bug report from Christoph, pointing out the things I should have done at the time. If you're interested and have the time, feel free to create a new branch and "fix" those issues / suggestions.

afilina added a commit to php/doc-base that referenced this pull request Oct 15, 2025
Needed to address this: php/doc-en#537
@afilina
Copy link
Contributor

afilina commented Oct 15, 2025

Closing in favor of a new PR which addresses the suggestions: #4930

@afilina afilina closed this Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants