-
Notifications
You must be signed in to change notification settings - Fork 60
Custom Event Refactor [AARD-2080]
#1275
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
base: dev
Are you sure you want to change the base?
Conversation
(because other open PRs add new events in ways that won't necessarily show up as conflicts, I'll want to do one last check before this is merged) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some non-trival conflicts here. Since this is not user facing this will not be put in for v7.2
.
Just merged |
Task
AARD-2080
Symptom
Currently there are a bunch of custom event classes with inconsistent implementations that both make it harder to find events and make the whole system more verbose. Unsubscription is also often done incorrectly or not done at all
Solution
I introduced an
EventSystem
class that manages all custom events and has genericcreate
,dispatch
, andlisten
methods.Verification
Go through and try and trigger the list of events found in
EventDataMap
and confirm that they still work as before. This PR should not affect functionalityBefore merging, ensure the following criteria are met: