-
-
Notifications
You must be signed in to change notification settings - Fork 135
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
BUGFIX: 3574 initialize foreign referenced nodes #3576
Conversation
Should we go with both PRs just to be one the safe side? |
No I rather not - I like to be more explicit and if we just ignore any unexpected input wrong behaviour will later be worse to debug then if there was an exception. we can still write a safeguard and log an error to the console if you want… |
Yes then I will adjust my PR to do that. The code there just relies too much on data being there. |
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.
I like! ❤️ I can't evaluate the technical solution, but it looks reasonable.
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.
LGTM code-wise.
However, I was unable to reproduce the issue as per description in #3574.
Maybe because we have the hotfix #3575 already merged? (So the console output should still complain, but less so) Also for future reviewers: Make sure to revert neos/neos-development-collection#4399 once its merged ;) |
Oh yes, didn't see that :) |
Co-authored-by: Wilhelm Behncke <2522299+grebaldi@users.noreply.github.com>
resolves: #3574
actual fix instead of hotfix: #3575 and neos/neos-development-collection#4399
similar to @grebaldi solution at: #3364
What I did
How I did it
How to verify it
revert: neos/neos-development-collection#4399 and try to follow #3574