Skip to content
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

Merged
merged 11 commits into from
Dec 20, 2024
Merged

Dev igd 45 #60

merged 11 commits into from
Dec 20, 2024

Conversation

donaldcampbelljr
Copy link
Member

Work towards solving #45 as well as adding better test coverage for IGD workflow

@donaldcampbelljr donaldcampbelljr changed the base branch from master to dev December 18, 2024 18:37
@donaldcampbelljr donaldcampbelljr marked this pull request as ready for review December 19, 2024 22:53
@donaldcampbelljr
Copy link
Member Author

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.

Copy link
Member

@nleroy917 nleroy917 left a 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 {
Copy link
Member

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"?

Copy link
Member Author

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.

@donaldcampbelljr
Copy link
Member Author

We can run cargo fmt on dev right before today's release.

@donaldcampbelljr donaldcampbelljr merged commit b33c233 into dev Dec 20, 2024
@donaldcampbelljr donaldcampbelljr deleted the dev_igd_45 branch December 20, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants