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

Added event handler for MemoryManager operations. #59

Merged
merged 6 commits into from
Jan 23, 2024

Conversation

fjbelizon
Copy link
Contributor

  • Added event handler for MemoryManager operations.

@fjbelizon fjbelizon added the enhancement New feature or request label Jan 22, 2024
@fjbelizon fjbelizon self-assigned this Jan 22, 2024
@fjbelizon fjbelizon linked an issue Jan 22, 2024 that may be closed by this pull request
@fjbelizon fjbelizon enabled auto-merge January 22, 2024 15:23
@LuisM000
Copy link
Contributor

We could simplify the code by sending the current instance of MemoryManager (i.e., 'this') as the sender when the event is raised. I notice that in most cases, the description of the generated event (MemoryStorageEventEnum.Upsert.GetEnumDescription()) is currently being sent as the sender. While I understand that there may be a specific reason for this, it's worth mentioning to ensure clarity :)

@rliberoff
Copy link
Member

We could simplify the code by sending the current instance of MemoryManager (i.e., 'this') as the sender when the event is raised. I notice that in most cases, the description of the generated event (MemoryStorageEventEnum.Upsert.GetEnumDescription()) is currently being sent as the sender. While I understand that there may be a specific reason for this, it's worth mentioning to ensure clarity :)

Perhaps add comments in the code to explain how this event is expected to work and be consumed. For instance, that the name of the event is sent to allow handlers to filter.

@fjbelizon
Copy link
Contributor Author

We could simplify the code by sending the current instance of MemoryManager (i.e., 'this') as the sender when the event is raised. I notice that in most cases, the description of the generated event (MemoryStorageEventEnum.Upsert.GetEnumDescription()) is currently being sent as the sender. While I understand that there may be a specific reason for this, it's worth mentioning to ensure clarity :)

There is no reason for this. Your arguments are logical and the event type is sent in the arguments, so it doesn't make sense to send the event type in the sender. I modify it to send the sender correctly.

@fjbelizon fjbelizon requested a review from rliberoff January 22, 2024 16:12
LuisM000
LuisM000 previously approved these changes Jan 22, 2024
VictorMuMo
VictorMuMo previously approved these changes Jan 22, 2024
@fjbelizon fjbelizon merged commit 4498a87 into main Jan 23, 2024
4 checks passed
@fjbelizon fjbelizon deleted the @fbelizon/event-handling-in-memory-storage branch January 23, 2024 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants