[ETCM-943] partially split reading part of Blockchain#1022
Merged
AurelienRichez merged 8 commits intodevelopfrom Jun 28, 2021
Merged
[ETCM-943] partially split reading part of Blockchain#1022AurelienRichez merged 8 commits intodevelopfrom
AurelienRichez merged 8 commits intodevelopfrom
Conversation
AurelienRichez
force-pushed
the
refactor/ETCM-943-split-reading-part
branch
from
7f44df2 to
13c47db
Compare
dzajkowski
reviewed
Jun 24, 2021
| } | ||
| import io.iohk.ethereum.mpt.MptNode | ||
|
|
||
| class BlockchainReader( |
Contributor
There was a problem hiding this comment.
would the next step be to create a trait BlockchainReader to allow for easier stubbing?
Contributor
Author
There was a problem hiding this comment.
Yes I agree. In the end we might split it further and have several reader traits.
dzajkowski
approved these changes
Jun 24, 2021
AurelienRichez
force-pushed
the
refactor/ETCM-943-split-reading-part
branch
from
da39ad5 to
a9a194a
Compare
leo-bogastry
approved these changes
Jun 25, 2021
added 7 commits
June 28, 2021 09:08
…o BlockchainReader
… BlockchainHostActor dependencies
AurelienRichez
force-pushed
the
refactor/ETCM-943-split-reading-part
branch
from
a9a194a to
e54ad4f
Compare
AnastasiiaL
approved these changes
Jun 28, 2021
AurelienRichez
force-pushed
the
refactor/ETCM-943-split-reading-part
branch
from
d3bc48d to
3b4a0cd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The goal of this PR is to put the read only part of
Blockchainin another class in order to reduce the dependency on this class. As a target I choose to makeBlockchainHostActorindependent ofBlockchain.I moved a few function inside a new class named
BlockchainReaderfor consistency, but we could come up with another name.It allowed to remove
Blockchainat some other places but there are still read only methods that can be moved to go further. But as it is already a big change we should at least validate this before continuing.review
This is a big change, but mostly moving things around. Each commit can be viewed one by one and is mergeable, so we can split this PR if needed
One important thing to think about is : does the change help us for a future refactoring ? I don't necessarily see
BlockchainReaderas a final target for the structure of the code, but more as step to help us refactor step by step.