Remove generics from test VM #1446
Conversation
test_vm/src/lib.rs
Outdated
| pub primitives: FakePrimitives, | ||
| pub store: &'bs BS, | ||
| pub store: &'bs TrackingMemBlockstore, |
There was a problem hiding this comment.
The MockVM uses an Rc<> vs TestVM using an & 'bs. Probably one of these patterns is better than the other. The Rc<> is flexible and removes some lifetime specifier noise from the code. Maybe we should do that here too? @Stebalien wdyt?
There was a problem hiding this comment.
Rc is generally fine, especially because the Blockstore trait is implemented for Rc<impl Blockstore> (so there's no need to manually call .borrow() everywhere.
And dropping the lifetime would also be nice.
There was a problem hiding this comment.
Ah, wait. I'm confusing RefCell and Rc. Yeah, there's no reason not to use Rc<TestBlockstore>.
There was a problem hiding this comment.
I went down this path but ended up running into some really weird Rust compiler errors. Are you okay doing this in a follow up PR ?
For now, I'd like to focus on and resolve #1445 (comment). Please can you leave your thoughts there ?
There was a problem hiding this comment.
I'm fine doing it in a followup, yes.
aarshkshah1992
changed the base branch from
feat/remove-mocktime-generic-blockstore
to
master
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #1446 +/- ##
=======================================
Coverage 91.15% 91.15%
=======================================
Files 146 146
Lines 27950 27951 +1
=======================================
+ Hits 25478 25479 +1
Misses 2472 2472
|
Remove generics from test VM.
Part 4 of #1442.