types: Drop model newtype on Rust primitive types#666
Conversation
There was a problem hiding this comment.
The changes in total look good, but the commit sequence is a bit confusing to me. For example when you go through them the first one changes a test, but why? the reason that the test needs to be changed hasn't happened yet. And patch 2 doesn't pass verify because the verify fix is in a later patch. The work as a whole is good though. I would be happy if it was one patch, but if you want more for personal/green dots in github reasons I am cool with that too, but they need to make sense in order.
Would be squashing right away! |
- Drop model::GetBlockCount type and into_model impl - Remove model verification from GetBlockCount test - Update getblockcount docs to version-only - Update GetBlockCount verification to no-model
- Drop model::GetDifficulty type and into_model impl - Remove model verification from GetDifficulty test - Update GetDifficulty docs to version-only - Update GetDifficulty verification to use no-model
48e8cc7 to
623a4ff
Compare
Abeeujah
changed the title
jamillambert
left a comment
jamillambert
left a comment
There was a problem hiding this comment.
- RescanBlockchain: both struct fields (start_height, stop_height) are
u32
They are both still i64 in types. I assume since everything worked with the conversion to the model where they were u32 means they can be changed from i64 to u32 in types.
- Drop model::RescanBlockchain type and into_model impl - Remove model verification from RescanBlockchain test - Update RescanBlockchain fields (start_height, stop_height) to be `u32` - Update RescanBlockchain docs to version-only - Update RescanBlockchain verification to use no-model
623a4ff to
e9dba92
Compare
2 participants
Drops the following types in model which do not model a concrete rust-bitcoin type
u64f64u32