-
Notifications
You must be signed in to change notification settings - Fork 75
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
chore(sequencer): consolidate all action handling to one module #1759
base: main
Are you sure you want to change the base?
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.
Could the unit tests for each action be moved from the common action_handler::tests
module back into the modules specific to each individual unit under test please?
Generally I think it's good practice to keep unit tests as close to the unit under test as possible, but it will probably also help with this review since we'll see mostly files being moved with minimal changes, whereas just now we see a lot of files being added and deleted.
I'm also not sure that we need the impls
submodule - I'd be inclined to just flatten this into action_handler
, but I don't feel very strongly about that.
Summary
Moved all action and transaction handling to single module.
Background
Previously, all implementations of
ActionHandler
were done separately in components which corresponded to their given actions (or groups of actions, such as inbridge
). While this may make sense from an action-centered point of view, it is confusing to an outsider to have to look many different places to find where stateless/stateful checks and execution occur. This PR consolidates all of the implementations as well as their testing.Changes
action_handler
module and moved the correspondingActionHandler
trait as well as all of itsimpl
s into this module.tests
.impls
.Testing
Passing all tests.
Changelogs
Changelogs updated
Related Issues
closes #1657