change command template to return actual exit code #1006
Conversation
|
I don't like how this is now not in sync with controllers. It also doesn't reflect reality IMO |
| */ | ||
| public function execute(Arguments $args, ConsoleIo $io) | ||
| public function execute(Arguments $args, ConsoleIo $io): int |
There was a problem hiding this comment.
| public function execute(Arguments $args, ConsoleIo $io): int | |
| public function execute(Arguments $args, ConsoleIo $io): ?int |
Wouldn't this be more correct?
There was a problem hiding this comment.
I would rather we keep it int only so that in a future release we can add int type to our base class Command::execute() itself.
There was a problem hiding this comment.
Wouldnt this start with core though
Baked Code by no means would be a reliable way of going into that direction
There was a problem hiding this comment.
This kind of change can only be enforced in core in 6.x anyway.
With this change in bake it at least encourages new commands being generated to use this kind of setup.
There was a problem hiding this comment.
Is requiring an explicit return value for commands going to make userland code better? I can see how it makes the framework code simpler, but does it also benefit end users?
There was a problem hiding this comment.
I very much doubt that.
That said: Symfony went into that direction and the new versions require a non void (maybe even integer) return code.
My point here is that bake is the end of the chain - and narrowing it here to just int seems not like the best idea if there was no broader discussion on core changes yet.
We return success on null as a fallback, sure, but this doesn't mean this doesn't reflect reality. I just don't like having 3 return types for a command which should always return an exit code since its a CLI tool. |
|
since there is not enough common agreement, this will not change in current Cake5. maybe something to consider for Cake6 |
Refs: #950