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 download status, added comparison by hash sum #474

Merged
merged 1 commit into from
Sep 18, 2023

Conversation

schtobia
Copy link
Contributor

This is a port from TheLartians/ModernCppStarter/pull/171. I have no idea if the workflow works correctly - tested it on my machine, but I couldn't figure out how to get this runner up on my fork. 😞

Copy link
Member

@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.

Looks like a good start! As for testing, you should enable workflows and the create a new release here to trigger the workflow and artifact upload.

cmake/get_cpm.cmake Outdated Show resolved Hide resolved
@schtobia
Copy link
Contributor Author

Looks like a good start! As for testing, you should enable workflows and the create a new release here to trigger the workflow and artifact upload.

Done, and I found a bug in the workflow, which is also fixed with this MR.

cmake/get_cpm.cmake Outdated Show resolved Hide resolved
cmake/get_cpm.cmake Outdated Show resolved Hide resolved
Copy link
Member

@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.

Sorry for taking so long to respond, but looks great so far! Looking forward to adding it to the project.

@schtobia schtobia force-pushed the master branch 2 times, most recently from 9772a4c to 3b28957 Compare July 13, 2023 03:53
@schtobia schtobia marked this pull request as ready for review August 7, 2023 18:33
@schtobia
Copy link
Contributor Author

schtobia commented Aug 7, 2023

IMHO this is ready for merging.

TheLartians
TheLartians previously approved these changes Aug 19, 2023
Copy link
Member

@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 changes! This looks great, lmk if you want to see if we can simplify the code bit by handling most logic with file(DOWNLOAD) or see flaws with my idea. Otherwise I'm also happy with merging this as is.

@schtobia
Copy link
Contributor Author

So, new try. What do you think?

Copy link
Member

@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.

This looks great, thanks again for the patience and updates!

@TheLartians TheLartians merged commit 699c7a0 into cpm-cmake:master Sep 18, 2023
8 checks passed
@TheLartians
Copy link
Member

The updated CPM download script has been released in v0.38.3! 🎉

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