Some refactors and command collector #4
Conversation
There was a problem hiding this comment.
@JustSteveKing just made two small suggestions.
I have a PR to create but I'll wait for you to merge your work first because it's related to the input and it depends on how you're going to build this initial stuff
| string $description, | ||
| string $instance, | ||
| ): void { | ||
| $this->commands[$signature] = [ |
There was a problem hiding this comment.
I think it's better to create a class to wrap these properties, so $commands can be an array of CommandWrapper or something like that.
There was a problem hiding this comment.
Yeah this is something I wasn't sure on - I like the idea though! Something like:
final class StoredCommand
{
public function __construct(
private readonly string $signature,
....
) {}
}There was a problem hiding this comment.
I like it, if you think that could be a nice thing, go for it!!!
src/Minicli.php
Outdated
| } | ||
| protected readonly Config $config; | ||
|
|
||
| public readonly CommandCollector $commands; |
There was a problem hiding this comment.
This could be protected or private IMO
There was a problem hiding this comment.
Yeah I agree. Typically I make things public while fleshing out the ideas, then test going through updating to private where possible
# Conflicts: # src/Commands/AbstractCommand.php # src/Commands/DefaultCommand.php # src/Minicli.php
The beginning of something interesting