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

Update README with downloading HPO data #47

Merged
merged 3 commits into from
Mar 9, 2024

Conversation

apraga
Copy link
Contributor

@apraga apraga commented Mar 6, 2024

Hi,

Just discovered your project and it looks great ! I was a bit confused about downloading the data, so here's a pull request to make it hopefully more clear on the README.

Thanks,

@anergictcell
Copy link
Owner

Great suggestions, thanks for the PR. I'll happily merge it, but the CICD has some minor errors.
They are not caused your changes, but are caused by the strict clippy and fmt rules I defined. The rust version was updated and apparently has new formatting requirements for rust fmt.

I am traveling without my laptop and can't make those fixes myself unfortunately. You could integrate them into your PR or wait for me to do it next week.

Also you might have to change the code blocks you added to the readme to no_run, otherwise the automated tests will fail.
Can you run

cargo fmt
cargo clippy
cargo test

Just to ensure it all works.

Sorry to be so pedantic, even for such a minor fix.
Thanks for your help

Minor changes. Documentation should be added for panics.
@apraga
Copy link
Contributor Author

apraga commented Mar 6, 2024

Hi,
cargo clippy complained about panics not documentated and a few other things. I've added some fixes to make it happy again.

Edit: I cannot reproduce the build errors locally. Any ideas ?

@anergictcell
Copy link
Owner

It seems that clippy also became more strict. Of you don't feel confide t implementing the suggested fixes, I will do that this weekend and merge your changes.
As nice as automated check for merging are, they can really become annoying for projects that don't have actual continuous pushes ;)

@anergictcell anergictcell changed the base branch from main to fmt-clippy-updates March 9, 2024 17:42
@anergictcell anergictcell merged commit 6c9a506 into anergictcell:fmt-clippy-updates Mar 9, 2024
1 check failed
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