Skip to content
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

Document lazy objects #3975

Merged
merged 8 commits into from
Nov 4, 2024
Merged

Document lazy objects #3975

merged 8 commits into from
Nov 4, 2024

Conversation

arnaud-lb
Copy link
Member

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.

Wow, that is a lot of new content! Thank you!

language/oop5/lazy-objects.xml Outdated Show resolved Hide resolved
language/oop5/lazy-objects.xml Outdated Show resolved Hide resolved
language/oop5/lazy-objects.xml Outdated Show resolved Hide resolved
$reflector = new ReflectionClass(Example::class);
$lazyObject = $reflector->newLazyGhost(function (Example $object) {
// Initialize object in-place
$object->__construct(1);
Copy link
Member

Choose a reason for hiding this comment

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

I hate seeing ->__construct(), but that likely inherent to lazy ghost (and irrelevant to this PR anyway).

language/oop5/lazy-objects.xml Outdated Show resolved Hide resolved
language/oop5/lazy-objects.xml Outdated Show resolved Hide resolved
language/oop5/lazy-objects.xml Outdated Show resolved Hide resolved
language/oop5/lazy-objects.xml Outdated Show resolved Hide resolved
language/oop5/lazy-objects.xml Outdated Show resolved Hide resolved
during the final initialization step). However, the proxy can be used
for decisions based on the values of initialized properties, the class,
the object itself, or its identity. For instance, the initializer might
use an initialized property’s value when creating the real instance.
Copy link
Member

Choose a reason for hiding this comment

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

I think we're apos elsewhere, so likely want to stick with it (for now):

Suggested change
use an initialized propertys value when creating the real instance.
use an initialized property's value when creating the real instance.

Oh, just noticed that you're using elsewhere, so maybe stick with that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I've been inconsistent with this. I will unify this. Which way is preferred?

Copy link
Member

Choose a reason for hiding this comment

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

The right way[TM] would be to use typographic apostrophes (i.e. ), but I assume that we mostly use plain apostrophes (i.e. ') so far. Maybe this can be changed at some point in the future without disturbing translators (i.e. skip-revcheck).

Perhaps @Crell has something to say about this.

Copy link
Contributor

Choose a reason for hiding this comment

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

My feeling is that in technical documentation, just use ' and be done with it. Messing with typographical apostrophes and quotes just creates hassles around character encoding. So just stick with the simple characters on your keyboard.

Copy link
Member

@Girgias Girgias 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 mainly got a bunch of markup nits which are copied from existing docs which are less then ideal but better to not introduce more of these. :)

language/oop5/lazy-objects.xml Outdated Show resolved Hide resolved
language/oop5/lazy-objects.xml Outdated Show resolved Hide resolved
language/oop5/lazy-objects.xml Outdated Show resolved Hide resolved
language/oop5/lazy-objects.xml Outdated Show resolved Hide resolved
language/oop5/lazy-objects.xml Outdated Show resolved Hide resolved
reference/reflection/reflectionclass/resetaslazyghost.xml Outdated Show resolved Hide resolved
reference/reflection/reflectionclass/resetaslazyproxy.xml Outdated Show resolved Hide resolved
reference/reflection/reflectionproperty/islazy.xml Outdated Show resolved Hide resolved
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Some final minor markup nits, and don't forget to address the ' apostrophe usage. :)

Comment on lines +19 to 27
<row>
<entry>8.4.0</entry>
<entry>
Added: Support for <link linkend="language.oop5.lazy-objects">Lazy Objects</link>.
</entry>
</row>
<row>
<entry>8.1.0</entry>
<entry>
Copy link
Member

Choose a reason for hiding this comment

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

Aside, we probably need to document about typing constants and readonly

reference/reflection/reflectionclass.xml Outdated Show resolved Hide resolved
reference/reflection/reflectionclass.xml Outdated Show resolved Hide resolved
reference/reflection/reflectionclass/newlazyproxy.xml Outdated Show resolved Hide resolved
reference/reflection/reflectionclass/newlazyproxy.xml Outdated Show resolved Hide resolved
reference/reflection/reflectionclass/resetaslazyproxy.xml Outdated Show resolved Hide resolved
reference/reflection/reflectionclass/resetaslazyproxy.xml Outdated Show resolved Hide resolved
Copy link
Member

@Girgias Girgias 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!

@Girgias Girgias merged commit c0fa507 into php:master Nov 4, 2024
2 checks passed
@Girgias Girgias added this to the PHP 8.4 milestone Nov 4, 2024
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