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

TASK: Remove "internal" node properties #45

Merged
merged 4 commits into from
Apr 12, 2024

Conversation

kitsunet
Copy link
Member

@kitsunet kitsunet commented Mar 7, 2024

Adds rector migrations for common "underscore" internal property access on nodes.

Related: #neos-development-collection/4208

Adds rector migrations for common "underscore" internal
property access on nodes.

Related: neos/#4208
@kitsunet
Copy link
Member Author

kitsunet commented Mar 7, 2024

Mmm, these test failures seem unrelated?

@ahaeslich
Copy link
Member

ahaeslich commented Mar 17, 2024

Mmm, these test failures seem unrelated?

Sure are. I stumbled upon them with my PR and as I investigated the last successful run on main was with this commit: d3cc877

@ahaeslich
Copy link
Member

Upon further investigation using phpstan/phpstan in version 1.10.13 the tests are green again on main.

Next step in this 🐰 hole: why can't we relax the requirement?

@dlubitz dlubitz self-requested a review March 17, 2024 09:01
Copy link
Contributor

@dlubitz dlubitz left a comment

Choose a reason for hiding this comment

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

@kitsunet Thanks for taking care. Could you please add the new cases to the existing tests?

E.g here for _depth:
https://github.com/neos/rector/blob/main/tests/ContentRepository90/Rules/FusionNodeDepthRector/Fixture/some_file.fusion.inc

@dlubitz dlubitz merged commit d7c832e into main Apr 12, 2024
2 checks passed
@mhsdesign mhsdesign deleted the task/4208-remove-internal-properties branch April 26, 2024 08:15
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