Conversation
|
I’m aware that this would also work without defining the task upfront. However, using empty() in this context does not feel semantically correct to me. |
| } elseif (is_array($body)) { | ||
| $task = new GroupTask($name, $body); | ||
| } else { | ||
| throw new \InvalidArgumentException('Task body should be a function or an array.'); |
There was a problem hiding this comment.
But we still need to do this check, no?
There was a problem hiding this comment.
I don't think so.
If $body is null and task does not exist then the Collection will throw not found. If $body is not of type callable|array|null then a fatal error should be thrown.
We also could transform the if to one line.
$task = is_callable($body) ? new Task($name, $body) : new GroupTask($name, $body)
There was a problem hiding this comment.
$task = is_callable($body) ? new Task($name, $body) : new GroupTask($name, $body)
Nope. Only arrays should be groups.
There was a problem hiding this comment.
I know and the only possbile type for the else is an array.
If the array is a callable then is_callable will be true and a Task will be created.
But maybe I'm overlooking something.
2 participants
I maintain several custom recipes for systems like Drupal, TYPO3, and Shopware. All of them share a common base recipe that defines our standard deploy task workflow. To make this setup more flexible, I would like to introduce an empty group task for
deploy:projectin the common recipe. This task can then be overridden by specific recipes, for example in a Symfony recipe. However, with the empty() check, the task is never registered; instead, an exception is thrown indicating that the task was not found.Since the task() function now uses typed parameters, throwing this exception is no longer necessary or possible. Therefore, I have removed it.
Nothing changed in the docs