neos / neos-development-collection

The unified repository containing the Neos core packages, used for Neos development.
https://www.neos.io/
GNU General Public License v3.0
260 stars 220 forks source link

!!! TASK: Deprecate setting NodeNames via cr commands #5082

Open mhsdesign opened 1 month ago

mhsdesign commented 1 month ago

Resolves: https://github.com/neos/neos-development-collection/issues/5050

Upgrade instructions

The parameter $nodeName is not accepted anymore in the main factory via CreateNodeAggregateWithNode::create and CopyNodesRecursively.

Please adjust your code to use the additional with*-er instead.

$command = CreateNodeAggregateWithNode::create(...);

if ($nodeName) {
    $command = $command->withNodeName($nodeName);
}

If you set their value previously simply to null, you can just omit setting this argument.

Review instructions

Checklist

nezaniel commented 4 days ago

I definitively prefer named arguments over withers...

mhsdesign commented 4 days ago

alternatively we can put succeedingSiblingNodeAggregateId at the end (after initialProperties) or just omit this change from this change ... it was just a thought which came up here: https://github.com/neos/neos-development-collection/pull/5082#discussion_r1607733756

i think we definitely want to mark setting the nodeName as deprecated as it is infact a deprecated feature.

nezaniel commented 4 days ago

imho node names become a deprecated feature once Neos starts resolving sites by id, not name. I'd agree to make nodename a wither, but not succeeding sibling id since this is a common use case (I'm literally using this at this moment in the starship)

mhsdesign commented 4 days ago

Okay fair enough, in that case i was maybe too quick ... anyways i reverted the change to introduce withSucceedingSiblingNodeAggregateId. This part is about the node names ;)