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

FEATURE: Rewrite NodeType::getName() #26

Merged
merged 3 commits into from
Sep 29, 2023

Conversation

dlubitz
Copy link
Contributor

@dlubitz dlubitz commented Sep 29, 2023

Rector migration for NodeType::getName removal

neos/neos-development-collection#4560

PHP
$nodeType->getName() to $nodeType->name->value

Fusion
${node.nodeType.name} to ${node.nodeTypeName.value}

Fixes partly #25

Copy link
Member

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

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

I love it, thanks! (+1 by reading)

just a mini question re comment positioning.

Also I wonder: Do all our rector migrations add a "TODO 9.0 migration" prefix?

@ahaeslich
Copy link
Member

Also I wonder: Do all our rector migrations add a "TODO 9.0 migration" prefix?

I remember a conversiation that we add this prefix if we can't be sure that the migration would be correct. E.g. if we can't be sure if it is a node.

Copy link
Member

@ahaeslich ahaeslich left a comment

Choose a reason for hiding this comment

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

@dlubitz shouldn't both rules be included in config/set/contentrepository-90.php as you did in #18?

@dlubitz
Copy link
Contributor Author

dlubitz commented Sep 29, 2023

@bwaidelich As Anke says, it's on all places where the migrator should have have a look and might need to change something manually.
@ahaeslich You are absolutly right. I added them.

@ahaeslich ahaeslich merged commit a58461c into main Sep 29, 2023
2 checks passed
@ahaeslich ahaeslich deleted the feature/replace-nodetype-getname branch September 30, 2023 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants