Skip to content

Stringable#478

Merged
cmb69 merged 8 commits intophp:masterfrom
Crell:stringable
Apr 1, 2021
Merged

Stringable#478
cmb69 merged 8 commits intophp:masterfrom
Crell:stringable

Conversation

@Crell
Copy link
Copy Markdown
Contributor

@Crell Crell commented Mar 4, 2021

I think that covers it, but please make sure I didn't miss a step...

Copy link
Copy Markdown
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! I think it is okay, but consider my comments.

Copy link
Copy Markdown
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.

Besides the missing stringable/tostring.xml, there are more issues:

  • commenting out the xi:includes accidentially commented out the </classsynopsis> tag
  • the stringable page needs to be added to languages/predefined/interfaces.xml (and then the methodsynopses should be included again (i.e. only comment out the contructorsysnopses)
  • there should be an entry in languages/predefined/versions.xml

Could you please take a look, @Crell?

@cmb69 cmb69 merged commit 069ce76 into php:master Apr 1, 2021
@Crell Crell deleted the stringable branch April 7, 2021 23:53
@jmikola
Copy link
Copy Markdown
Member

jmikola commented Nov 10, 2021

@cmb69: I just came across https://www.php.net/stringable and looking for a reason that the __toString() method was not documented as an interface method led me back to this PR. Was that intentional?

I noted you referred to "commenting out the xi:includes" in your previous comment but it still wasn't clear to me what you meant by that, as I didn't find discussion about commenting that out in any previously resolved comments. Perhaps it wasn't possible to link to magic.xml definition of the __toString() method directly form this context so the alternative was just to leave the list of methods empty?

@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Nov 10, 2021

[…] and looking for a reason that the __toString() method was not documented as an interface method […]

That documentation needs yet to be written (the PR has a respective comment); until this file is in place, the <xi:include> should be commented out, because otherwise the build system shows an error message (this should be fixed at some point, if possible). The linking to the magic methods doesn't work (I've overlooked that when approving), because <function>__toString</function> is not recognized by the build system; that should have been <link linkend="object.tostring">__toString()</link> instead.

Anyhow, a PR would be welcome!

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.

3 participants