-
Notifications
You must be signed in to change notification settings - Fork 247
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
Handle custom type serialization in snapshot source type #165
Handle custom type serialization in snapshot source type #165
Conversation
…ype for sanpshot source type
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.
Thanks for the pull request @jccf091.
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.
Thanks for the pull request @jccf091. Will you be able to make the small change I've outlined.
@@ -406,7 406,7 @@ defmodule Commanded.Aggregates.Aggregate do | |||
snapshot = %SnapshotData{ | |||
source_uuid: aggregate_uuid, | |||
source_version: aggregate_version, | |||
source_type: Atom.to_string(aggregate_module), | |||
source_type: TypeProvider.to_string(aggregate_module), |
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.
This breaks the contract specified by the Commanded.EventStore.TypeProvider
behaviour since it expects a struct
to be provided, not the module atom.
Instead, you can just pass in the aggregate_state
.
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.
Will do.
Thank for noticing that I break the behavior defined by the TypeProvider.
Is there anything you want to change? |
Sorry, forgot to merge in after your changes. Thanks @jccf091. |
No description provided.