New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding parents and children helper APIs to MerkleReg #116
Adding parents and children helper APIs to MerkleReg #116
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, comments are mostly nits
src/merkle_reg.rs
Outdated
use quickcheck::{Arbitrary, Gen}; | ||
use serde::{Deserialize, Serialize}; | ||
use std::collections::{BTreeMap, BTreeSet}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please keep the std/core imports up in the first group of imports, third party imports in the second group and local crate imports in the third group
src/merkle_reg.rs
Outdated
@@ -11,7 +11,7 @@ use crate::traits::{CmRDT, CvRDT}; | |||
pub type Hash = [u8; 32]; | |||
|
|||
/// A node in the Merkle DAG | |||
#[derive(Debug, Clone, PartialEq, Eq)] | |||
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, PartialOrd, Deserialize)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're going to derive PartialOrd, might as well get Ord as well. Also keep the serde(or any third party) derives together at the end.
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, PartialOrd, Deserialize)] | |
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)] |
@@ -57,7 +57,7 @@ impl<'a, T> Content<'a, T> { | |||
|
|||
/// Iterate over the Merkle DAG nodes holding the content values. | |||
pub fn nodes(&self) -> impl Iterator<Item = &Node<T>> { | |||
self.nodes.values().map(|n| *n) | |||
self.nodes.values().copied() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
src/merkle_reg.rs
Outdated
@@ -74,7 +74,7 @@ impl<'a, T> Content<'a, T> { | |||
/// The MerkleReg is a Register CRDT that uses the Merkle DAG | |||
/// structure to track the current value(s) held by this register. | |||
/// The leaves of the Merkle DAG are the current values. | |||
#[derive(Debug, PartialEq, Eq, Clone)] | |||
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, PartialOrd)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, PartialOrd)] | |
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)] |
src/merkle_reg.rs
Outdated
node.parents | ||
.iter() | ||
.copied() | ||
.filter_map(|leaf| self.dag.get(&leaf).map(|node| (leaf, node))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.filter_map(|leaf| self.dag.get(&leaf).map(|node| (leaf, node))) | |
.filter_map(|parent_hash| self.dag.get(&parent_hash).map(|node| (parent_hash, node))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚂
No description provided.