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

feat(UserMountCache): Emit events for added, removed and updated mounts #50157

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

provokateurin
Copy link
Member

Summary

Checklist

@provokateurin provokateurin added the pending documentation This pull request needs an associated documentation update label Jan 20, 2025
@provokateurin provokateurin force-pushed the feat/mountmanager/emit-events branch from e3b1f01 to d8c889d Compare January 20, 2025 14:05
@provokateurin provokateurin changed the title feat(MountManager): Emit events when mounts are added, removed and moved feat(UserMountCache): Emit events for added, removed and updated mounts Jan 20, 2025
@provokateurin provokateurin requested a review from come-nc January 20, 2025 14:05
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

Small question but otherwise looks promising

Comment on lines +132 to +134
foreach ($changedMounts as $mountPair) {
$this->eventDispatcher->dispatchTyped(new UserMountUpdatedEvent($mountPair[0], $mountPair[1]));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

$key is not needed in the event? It’s not clear to me what the string key in $changedMounts is.

Copy link
Member Author

@provokateurin provokateurin Jan 20, 2025

Choose a reason for hiding this comment

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

True, I only realized this myself after pushing. I can remove later, once the other parts are ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement pending documentation This pull request needs an associated documentation update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants