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

[9.x] Allow registering instances of commands #43986

Merged
merged 2 commits into from
Sep 5, 2022
Merged

Conversation

freekmurze
Copy link
Contributor

@freekmurze freekmurze commented Sep 2, 2022

Right now, you can only pass class names (or an array of class names) to resolveCommands. With the change in this PR, you can also pass instances of commands.

This change is needed power this upcoming feature in laravel-package-tools. Under the hood this works by instantiating a command, dynamically setting its signature, and registering the instantiated command (here, and here).

I didn't find any relevant tests around this, so I added none. Happy to add a test if you point me in the right direction. I did manually test it in a fresh Laravel app though.

@@ -273,6 +273,10 @@ public function resolve($command)
return null;
}

if ($command instanceof Command) {
Copy link
Member

Choose a reason for hiding this comment

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

@freekmurze Can adjust the PHP Annotations of the resolve method, and add a test on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Annotation adjusted. Like said in the PR description, I didn't find any meaningful tests for this method to build on.

@nunomaduro nunomaduro changed the title Allow registering instances of commands [9.x] Allow registering instances of commands Sep 3, 2022
@taylorotwell taylorotwell merged commit 0c18029 into laravel:9.x Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants