Skip to content

Conversation

@Jakuje
Copy link
Collaborator

@Jakuje Jakuje commented Nov 26, 2025

Follow-up from #311 and discussion in #326, where we agreed that keeping track of main is less problematic than keeping outdated version.

Copy link
Member

@hug-dev hug-dev left a comment

Choose a reason for hiding this comment

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

Thank you for the quick fix!

@Jakuje Jakuje requested a review from wiktor-k November 26, 2025 15:50
Copy link
Collaborator

@wiktor-k wiktor-k left a comment

Choose a reason for hiding this comment

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

I think this looks good 👌

OUT_DIR: /__w/rust-cryptoki/kryoptic/target/debug/deps/
RUST_BACKTRACE: 1
run: cargo test
run: cargo build --all-features && cargo test
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think check is enough, since we don't care about the output artifacts:

Suggested change
run: cargo build --all-features && cargo test
run: cargo check --all-features && cargo test

The question is whether we want to check --all-targets (which include tests and examples and bins...)...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I took this from ci.yml (where I probably wrote this previously). I think that just test should be enough. Not sure if we should need the check at all here as the check is executed in the other jobs (as well as the --all-targets -- the Fedora cargo should not be that much different from ubuntu cargo.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point 👍

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