-
Notifications
You must be signed in to change notification settings - Fork 7
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
Graph #42
Graph #42
Conversation
src/graph.rs
Outdated
pub fn determine_if_included( | ||
db: &Connection, | ||
earlier_unit: &String, | ||
later_units: &[&String], |
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.
this should be &[String] or you have construct a ref vec which is unnecessary
.iter() | ||
.map(|s| format!("'{}'", s)) | ||
.collect::<Vec<_>>() | ||
.join(", "); |
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.
you construct start_unit_list from star_unit which seems wrong, should be construct from later_units, or you have wrongly initialized start unit.
src/graph.rs
Outdated
new_start_units.dedup(); | ||
start_units = new_start_units; | ||
} else { | ||
break 'go_up; |
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.
here should be break with no label or return Ok(false). break 'go_up' we will have a infinite loop!
determine_if_included(db, earlier_unit, later_units) | ||
} | ||
|
||
pub fn read_descendant_units_by_authors_before_mc_index( |
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.
recheck the issues in determineIfIncluded
src/graph.rs
Outdated
if new_start_units.len() > 0 { | ||
start_units = new_start_units; | ||
} else { | ||
break 'go_down; |
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.
change this, find all the break with a label, and correct them
src/graph.rs
Outdated
db: &Connection, | ||
earlier_unit: String, | ||
later_units: &[String], | ||
) -> Result<bool> { |
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.
you should prefix unused variables with _ to remove warnings
Leave storage.read_props_of_units() as unimplemented.