-
Notifications
You must be signed in to change notification settings - Fork 2
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
Dev igd 45 #60
Dev igd 45 #60
Conversation
…or test assertions
…ery, remove local tests
This should fix issue #45. I also spent time going start to finish using igd create and then search and re-doing the tests to ensure that we have better coverage on both the create and search workflows. |
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.
Yeah this looks good, maybe just run cargo fmt
?
} | ||
|
||
/// Creates IGD database from a directory of bed files. | ||
pub fn create_igd_f(output_path: &String, filelist: &String, db_output_name: &String) { | ||
pub fn create_igd_f(output_path: &String, filelist: &String, db_output_name: &String) -> igd_t { |
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.
Is igd_t
a struct? What do you think about capitalizing it to be more "Rusty"?
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.
At some point we can do a large refactor to tackle things like this. For example, some variables are named in a way that are not super clear. This is because they were ported directly from C code and I wanted to keep them aligned for troubleshooting purposes.
We can run cargo fmt on dev right before today's release. |
Work towards solving #45 as well as adding better test coverage for IGD workflow