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

add C++ linking test #1971

Merged
merged 1 commit into from
Nov 1, 2024
Merged

Conversation

aidenfoxivey
Copy link
Contributor

Already worked on this code before - just added the variable to compile with Dilithium-2 support. It's already run in my local version of the repo.

CleanShot 2024-10-31 at 12 21 52@2x

It should be good to merge - but maybe one more glance over would be ideal.

Signed-off-by: Aiden Fox Ivey <aiden@aidenfoxivey.com>
Copy link
Member

@baentsch baentsch left a comment

Choose a reason for hiding this comment

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

This time the test ran. So LGTM for now.

As consideration for a future update: Change the test to run with a more "long term viable" algorithm (dilithium will most likely be removed at some point in the future which will make the test become stale).

Thanks for the contribution and seeing this through, @aidenfoxivey !

Question to @ryjones : Why did CI run this time without problems?

@dstebila
Copy link
Member

This time the test ran. So LGTM for now.

As consideration for a future update: Change the test to run with a more "long term viable" algorithm (dilithium will most likely be removed at some point in the future which will make the test become stale).

Thanks for the contribution and seeing this through, @aidenfoxivey !

Question to @ryjones : Why did CI run this time without problems?

I clicked "Approve and run"

@aidenfoxivey
Copy link
Contributor Author

This time the test ran. So LGTM for now.

As consideration for a future update: Change the test to run with a more "long term viable" algorithm (dilithium will most likely be removed at some point in the future which will make the test become stale).

Thanks for the contribution and seeing this through, @aidenfoxivey !

Question to @ryjones : Why did CI run this time without problems?

I'll make a future PR to change from using Dilithium to something else. I just copied this from the one test I found in C.

@dstebila dstebila merged commit 05257da into open-quantum-safe:main Nov 1, 2024
73 checks passed
@aidenfoxivey aidenfoxivey deleted the add-cpp-test branch November 2, 2024 02:28
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.

3 participants