-
Notifications
You must be signed in to change notification settings - Fork 75
[ETCM-939] refactor Ledger (by removing it) #1026
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
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.
Looks good overall, aside from some commented out code that could be removed
A description of what you did with the stuff that was in the Ledger would be nice |
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.
nice to see tests simplified 👍
src/main/scala/io/iohk/ethereum/testmode/TestModeComponentsProvider.scala
Show resolved
Hide resolved
src/test/scala/io/iohk/ethereum/blockchain/sync/FastSyncSpec.scala
Outdated
Show resolved
Hide resolved
src/test/scala/io/iohk/ethereum/blockchain/sync/ScenarioSetup.scala
Outdated
Show resolved
Hide resolved
src/test/scala/io/iohk/ethereum/blockchain/sync/regular/RegularSyncSpec.scala
Outdated
Show resolved
Hide resolved
Basically it was a layer that didn't add much except for complexity. I looked at the dependents and made the following changes:
The types aliases and case classes were moved to a package object and their own files respectively. |
It's been a journey; it looks easy enough on the
src/main
side but lots of stuff insrc/test
unfortunately broke along the way. The commits mostly tell a story of getting more tests passing while moving stuff out ofLedger
gradually.