- Notifications
You must be signed in to change notification settings - Fork11.7k
[11.x] Defer registering schedule registered usingApplicationBuilder::withSchedule()#51364
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
Plytas commentedMay 10, 2024
Thanks for your input@crynobone, but this doesn't solve the full problem. The problem still exists if you use just This change introduces a new issue where you cannot use both Note: |
crynobone commentedMay 10, 2024
You should defer resolving code dedicated for Schedule if |
Plytas commentedMay 10, 2024
You are correct, I forgot the fact that So the only issue I see now is that both methods cannot be used simultaneously while it could have been before. |
crynobone commentedMay 10, 2024
As in Laravel 10? It should only be possible to register Schedule within |
Plytas commentedMay 10, 2024
As in Laravel 11 before this change. I'll re-itterate my previous message. This change introduces a new issue where you cannot schedule commands in both Note: With this change |
`ApplicationBuilder::withScheduling()`fixes#51354Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
taylorotwell commentedJun 6, 2024
I'm pretty concerned about this breaking other applications. |
Plytas commentedJun 6, 2024
I've been thinking how to solve it and I like this approach if it didn't have that drawback. It feels like it would need a different hook to somehow call the callback only when actually calling schedule commands? Or somehow have scheduled commands inside |
crynobone commentedJun 7, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The only possible breaking change is when someone uses return Application::configure(basePath:dirname(__DIR__)) ->withSchedule(function ($schedule) { Artisan::command('go',fn () =>$this->comment('Done!'))->hourly(); })->create(); With this PR, |
crynobone commentedJun 11, 2024
Add additional tests to cover the limitation when adding new artisan commands within |
ApplicationBuilder::withScheduling()ApplicationBuilder::withSchedule()taylorotwell commentedJun 18, 2024
I dunno, I'm not comfortable merging this without much more community feedback and input. |




fixes#51354