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

Added a download status #171

Merged
merged 6 commits into from
Sep 18, 2023
Merged

Added a download status #171

merged 6 commits into from
Sep 18, 2023

Conversation

schtobia
Copy link
Contributor

This MR would a download status for and a more comprehensive compare against a given checksum for CPM.

Additionally, the versions of the pulled libraries is bumped.

Copy link
Owner

@TheLartians TheLartians left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, the status and checksum checks are definitely a welcome improvement!
Unfortunately they would need to be updated for each new version of CPM.cmake. It actually would be awesome to update the "official" get_cpm.cmake script (with values filled in in the publish GitHub workflow) to include the checksum as well!

@TheLartians
Copy link
Owner

@schtobia the code style status check is failing. Could you run the code formatter and update the PR?

@schtobia
Copy link
Contributor Author

It actually would be awesome to update the "official" get_cpm.cmake script (with values filled in in the publish GitHub workflow) to include the checksum as well!

You're absolutely right. I will work on this, but have no ETA.

This is very useful in shaky network solutions. We want both a success
and a failure message, to determine if the download was successful.
This is more comprehensive than just comparing against an empty file.
Copy link
Owner

@TheLartians TheLartians left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've gone ahead and updated to the new get_cpm.cmake script from the CPM project. Thanks again for the PR in both projects!

@TheLartians TheLartians merged commit 776d4f3 into TheLartians:master Sep 18, 2023
6 checks passed
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