statamic / ideas

💡Discussions on ideas and feature requests for Statamic
https://statamic.dev
32 stars 1 forks source link

Define parameter when doing schedules in addons #912

Closed robbanl closed 2 years ago

robbanl commented 2 years ago

Reference: https://github.com/statamic/cms/blob/3.3/src/Providers/AddonServiceProvider.php#L542

Shouldn't this be

protected function schedule(Schedule $schedule)

instead of current

protected function schedule($schedule)

This way I can easily see in my own addon the different schedule methods.

image

Right now I cannot use this since my method needs to be compatible with parent function.

andjsch commented 2 years ago

That has nothing to do with Statamic – you are in Laravel land right now. And you are basically right, but you already gave yourself the answer - you have to comply with the interface/parent class in order for this to work.

Will be possible with Laravel 10, see here

robbanl commented 2 years ago

Hmm, but is it really?

AddonServiceProvider extends Illuminate\Support\ServiceProvider and that class doesn't have a method called schedule?

Adding Schedule to my class give me:

Declaration of Codepeak\MyAddon\ServiceProvider::schedule(Illuminate\Console\Scheduling\Schedule $schedule) must be compatible with Statamic\Providers\AddonServiceProvider::schedule($schedule)

And if I update AddonServiceProvider with type declaration everything works. Or am I missing something more far back? :-)

andjsch commented 2 years ago

Nope, sorry, my bad. You are right.

If you want to, you can make a PR for it to get added. I think that would be faster.

robbanl commented 2 years ago

Great! :-)

Created this PR https://github.com/statamic/cms/pull/7081 and linked this discussion.

andjsch commented 2 years ago

Great! Might not be merged instantly. It's a breaking change for addons that already use the function! But still good to open up. I like it!