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

feat: supports SHA-3 and BLAKE2 as checksum algorithms #1067

Merged
merged 4 commits into from
May 20, 2024

Conversation

sorairolake
Copy link
Contributor

Closes #1056

Copy link
Member

@ashleygwilliams ashleygwilliams left a comment

Choose a reason for hiding this comment

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

hey @sorairolake! this looks super cool! i think the implementation looks accurate. to get this merged we will want to have this in some of our snapshot tests, just to make sure that we are generating the right files. are you familiar with snapshot tests and cargo-insta? if not feel free to hop in our discord and i can walk you through it! https://discord.gg/uJ5htJXV

@sorairolake
Copy link
Contributor Author

@ashleygwilliams Updated snapshots. However, I don't know much about the insta, so this may be wrong.

@ashleygwilliams
Copy link
Member

hey @sorairolake this looks good! for this feature, the main thing you'd want to see is that the checksum artifact was being created with the right algorithm, the right file extension, and it was reported correctly in the release body

looking at https://github.com/axodotdev/cargo-dist/pull/1067/files#diff-78049aebe4b03ce7a67f47b4f1f21f459dd4a6d9b890bd0f81fb0eb1364cd8edR1003-R1019 i think you've nailed it!

@sorairolake
Copy link
Contributor Author

@ashleygwilliams Thanks. Is there anything else that needs to be fixed?

@ashleygwilliams
Copy link
Member

@sorairolake i think this looks good! we have some folks out sick/on vacation, but once i get another team member to take a peak i'll know if we need anything else or if we can simply get this merged! very excited and thanks so much again for contributing!!

@ashleygwilliams ashleygwilliams merged commit 9f6ae53 into axodotdev:main May 20, 2024
15 checks passed
@ashleygwilliams
Copy link
Member

got a second set of eyes- looks good! thank you so much again!! btw- if you are at all interested in a followup - this issue might be an interesting one for you: #439. at the very least, i'd be curious to get your feedback on it and hear about how you use cargo-dist! thank you so much again for your contributions!!

@sorairolake sorairolake deleted the more-checksum-algorithms branch May 20, 2024 19:56
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.

Supports SHA-3 and BLAKE2 as checksum algorithms
2 participants