-
-
Notifications
You must be signed in to change notification settings - Fork 222
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
TASK: docs and readability of Node::getProperty
#4328
Conversation
might get obsolete when we opt in for @bwaidelich's cleanup #4304 (comment) / or can be build upon |
Neos.ContentRepository.Core/Classes/Projection/ContentGraph/PropertyCollection.php
Outdated
Show resolved
Hide resolved
You're right. I was assuming `isset` will return true if a key exists.
|
Node::getProperty
4738883
to
fcdfae7
Compare
@mhsdesign you have replaced the |
359e8da
to
cc9a45e
Compare
cc9a45e
to
c41d5fe
Compare
Rebased and removed all the crazy parts ;) so now its only improved docs and types and stuff ^^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 by 👀
This pr incorporates a few cleanups regarding
node.properties
Clean up how
node::getProperty
andhasProperty
access the lower level methods.Make the code more obvious - calling
node::getProperty
andhasProperty
goes into deep rabbit hole until we find the actual implementation. This makes it harder to understand the actual behavior. This also lead to slightly miss documented code:For example
hasProperty
might be true for a property even if it is null, because a serializer might unserialize it to null!I also adjusted the documentation (meta #4478)
As its was actually the same from the old cr and we dont have those funny content object dingsis anymore
Upgrade instructions
Review instructions
Checklist
FEATURE|TASK|BUGFIX
!!!
and have upgrade-instructions