Skip to content

Change mock implementation to use store.KVStore instead of maps - #146

Merged
tzdybal merged 13 commits into
evstack:mainfrom
jbowen93:revert-2-revert-1-disk_storage_mock_DALC_134
Oct 10, 2021
Merged

Change mock implementation to use store.KVStore instead of maps#146
tzdybal merged 13 commits into
evstack:mainfrom
jbowen93:revert-2-revert-1-disk_storage_mock_DALC_134

Conversation

@jbowen93

@jbowen93jbowen93 commented Oct 9, 2021

Copy link
Copy Markdown
Contributor

Resolves#134

Apologies for messed up commit history, need to make sure to squash before merging!

@codecov-commenter

codecov-commenter commented Oct 9, 2021•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #146 (bb0953c) into main (8a15588) will decrease coverage by 0.39%.
The diff coverage is 57.97%.

Impacted file tree graph

@@ Coverage Diff @@## main #146 +/- ##
==========================================
- Coverage 57.47% 57.07% -0.40% 
==========================================
Files 24 24 Lines 1987 2127 +140 ==========================================
+ Hits 1142 1214 +72 - Misses 729 786 +57 - Partials 116 127 +11
Impacted FilesCoverage Δ
state/executor.go69.44% <ø> (ø)
types/serialization.go58.82% <0.00%> (-2.91%)⬇️
node/node.go52.87% <52.56%> (-6.95%)⬇️
da/mock/mock.go68.96% <52.63%> (-31.04%)⬇️
p2p/client.go67.97% <57.14%> (+0.50%)⬆️
p2p/gossip.go85.24% <91.66%> (+3.42%)⬆️
node/aggregator.go68.62% <100.00%> (+1.62%)⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e58172...bb0953c. Read the comment docs.

@jbowen93

jbowen93 commented Oct 9, 2021

Copy link
Copy Markdown
ContributorAuthor

@tzdybal I don't see an option for me to assign a Reviewer so pinging you here.

tzdybal
tzdybal reviewed Oct 9, 2021
View reviewed changes
Comment threadtypes/serialization.go Outdated
tzdybal
tzdybal reviewed Oct 9, 2021
View reviewed changes
Comment threadtypes/serialization.go Outdated
tzdybal
tzdybal reviewed Oct 9, 2021
View reviewed changes

@tzdybaltzdybal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your submission!
This looks really good - I left some small comments.

@tzdybaltzdybal mentioned this pull request Oct 9, 2021
Closed
3 tasks
tzdybal
tzdybal approved these changes Oct 9, 2021
View reviewed changes

@tzdybaltzdybal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment threadda/mock/mock.go Outdated
Comment threadda/mock/mock.go Outdated
Comment threadda/mock/mock.go Outdated
@jbowen93@tzdybal
Apply suggestions from code review
bb0953c
Co-authored-by: Tomasz Zdybał <tomek@zdybal.lap.pl>
@tzdybal
tzdybal merged commit 59ab23b into evstack:mainOct 10, 2021
tzdybal pushed a commit that referenced this pull request Oct 12, 2021
@jbowen93@tzdybal
Change mock implementation to use store.KVStore instead of maps (#146)
3b23421
* update `MockDataAvailabilityLayerClient` to use `KVStore` instead of maps
* ensure that `KVStore` is passed to `Init` in the tests
Resolves#134.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adlerjohnadlerjohnAwaiting requested review from adlerjohn
@liamsiliamsiAwaiting requested review from liamsi
@mattdfmattdfAwaiting requested review from mattdf
1 more reviewer
@tzdybaltzdybaltzdybal approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add disk storage to mock DALC

3 participants

@jbowen93@codecov-commenter@tzdybal