fix/get: add missing api to AppendOnlyData trait #121
Conversation
Getting Entry by index is missing, but exists for Owner and Permissions. REFERENCE: maidsafe#119
|
Hmm. It is failing Travis due to code that did not change in this PR. Not sure why. |
It's from here @oetyng. |
There is unnecessary duplication of code in the new `get_entry` test.
|
Dismissed by push, sorry @m-cat :) |
|
Oh not again, I thought I would make it before you reviewed! |
src/append_only_data.rs
Outdated
| @@ -1398,11 +1398,11 @@ mod tests { | |||
| for data in data_vec { | |||
| assert_eq!( | |||
| data.entry(Index::FromStart(0)), | |||
| Some(&Entry::new(b"key0".to_vec(), b"value0".to_vec())) | |||
| Some(&entries.clone()[0]) | |||
There was a problem hiding this comment.
Won't this clone the entire vec? I think entries[0].clone() is what you want, but I haven't tested it.
Of course, this is just a test so the performance here isn't important.
There was a problem hiding this comment.
You're right! And actually, no need for clone at all there I realised. I was first thinking that the cloned entries vec was a shallow clone, and that the entries within needed to be cloned. But apparently cloning the vec must have cloned the contents as well?
There was a problem hiding this comment.
If the entries inside the Vec are a Clone type, then they will be cloned (even if they are a reference), otherwise if they are a reference but not a Clone type the references themselves will be cloned (shallow clone), otherwise you get a compiler error
No worries 🙂 Looks good, but I'll wait this time until the CI issues are fixed |
|
These changes look good. CI is failing because of rust-lang/rust-clippy#4326. |
|
Closing this since this data type is being replaced with |
Getting Entry by index is missing, but exists for Owner and Permissions.
REFERENCE: #119