Skip to content

Conversation

@zzorba
Copy link
Collaborator

@zzorba zzorba commented Sep 15, 2025

No description provided.

@zzorba zzorba force-pushed the fix_ci_cargo_version branch from 605b32b to a1aa7fc Compare September 15, 2025 14:11
@zzorba zzorba requested a review from jhugman September 15, 2025 16:58
@zzorba
Copy link
Collaborator Author

zzorba commented Sep 15, 2025

@jhugman this fixes one of the two failing tests @main, but not the bob one. Still worth reviewing + merging I believe.

Comment on lines +19 to +24
- name: Install Rust
uses: dtolnay/[email protected]

- name: Install Rust Fmt and Clippy
shell: bash
run: rustup component add rustfmt clippy
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we want to also enforce this outside the CI, we could use the rust-toolchain.toml file. If I understand the documentation correctly, rustup should then automatically install / pick the specified version when commands are issued.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, we don't have that flow in this repo just yet... where would I add it? @jhugman curious to get your thoughts on this also.

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 it's just a file at the repo level and rustup should use it automatically. I've never used it myself so far though. Not a real issue. Was just a thought.

Copy link
Owner

Choose a reason for hiding this comment

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

I think @Johennes is right; if we don't want to do that, then we can put it in the manifest.

Copy link
Owner

@jhugman jhugman 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 now that @Johennes has fixed the other issues, this should build cleanly. Good work team!

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.

4 participants