-
Notifications
You must be signed in to change notification settings - Fork 155
ci: Also validate the composefs-backend feature #1606
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
Conversation
I was hitting a compliation error which *looks* like it was actaully an incremental compilation bug? Or it might have been rust-analyzer and local builds fighting over enabled features. Anyways, this ensures that we're gating on the composefs backend compiling. Signed-off-by: Colin Walters <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request correctly adds CI validation for the composefs-backend feature, which is a great step to ensure build stability. The associated changes in the Rust code to conditionally compile constants are appropriate and well-executed. I have one minor suggestion for the Makefile to improve the clarity of the build commands.
| (cd crates/ostree-ext && cargo check --no-default-features) | ||
| (cd crates/lib && cargo check --no-default-features) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
|
|
||
| /// Wrapper around [`rustix::openat`] | ||
| /// Wrapper around [`rustix::fs::openat`] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accidentally included this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope it's intentional, make validate-rust trips on it. See https://docs.rs/rustix/latest/rustix/fs/fn.openat.html
|
Hm is something wrong with the repo config? I have the "Merge pull request" button but there's not an approval on this yet. I won't touch it for the moment just incase we need this state to troubleshoot... |
|
branch protection doesn't apply to anything other than main right now |
I was hitting a compliation error which looks
like it was actaully an incremental compilation bug? Or it might have been rust-analyzer and local builds fighting over enabled features.
Anyways, this ensures that we're gating on the composefs backend compiling.