-
Notifications
You must be signed in to change notification settings - Fork 93
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
Move settlement updater #3018
Move settlement updater #3018
Conversation
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.
I like the new naming.
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.
nit: The "to be renamed component" is the only one that creates settlement::Settlement
instances (the infra
is the only one consuming it). That means settlement::Settlement::new()
can now be made private. That way we ensure that we don't have other components create these types by accident.
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
Description
OnSettlementEventUpdater
was previously refactored and ended up being dependent ondomain
andinfra
objects only.That means it is ready to be moved to a proper place inside
domain
.Changes
How to test
Existing e2e tests.