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

[5.8] Fix event:list command #28624

Merged
merged 1 commit into from May 27, 2019

Conversation

Projects
None yet
3 participants
@joedixon
Copy link
Contributor

commented May 27, 2019

When using a combination of manually registering events in the EventServiceProvider and event auto discovery, the output of php artisan event:list can be unexpected.

Example
If I have two listeners in the app/Listeners directory for the VoteWasCast event (e.g. StoreVote and SendVote), but I only have one of those manually registered in the EventServiceProvider as below, I would still expect both to be included in the output of php artisan event:list.

class EventServiceProvider extends ServiceProvider
{
    protected $listen = [
        VoteWasCast::class => [
            StoreVote::class,
        ]
    ];
}

However, below is the output I get:

Screenshot 2019-05-27 at 09 49 31

This PR provides the following output which is in line with what I would expect.

Screenshot 2019-05-27 at 09 51 01

@driesvints driesvints changed the title Fix event:list command [5.8] Fix event:list command May 27, 2019

@driesvints

This comment has been minimized.

Copy link
Member

commented May 27, 2019

Isn't it weird that the StoreVote class is now listed twice?

@joedixon

This comment has been minimized.

Copy link
Contributor Author

commented May 27, 2019

Well, I think it’s probably expected behaviour. If I add it to the $listen array and have auto discovery on, my listener fires twice. I assume it’s the developers responsibility not to list it there and use auto discovery?

@driesvints

This comment has been minimized.

Copy link
Member

commented May 27, 2019

@joedixon ah, if it's fired twice than that definitely makes sense. I can follow that reasoning 👍

@taylorotwell taylorotwell merged commit 76703ab into laravel:5.8 May 27, 2019

2 checks passed

continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.