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

Use bootc-utils for command helpers #821

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

cgwalters
Copy link
Member

This enables more code sharing. I didn't do a full port here, but this starts the ball rolling. In particular I think run_and_parse_json() is nice and elegant.

This enables more code sharing. I didn't do a full port
here, but this starts the ball rolling. In particular
I think `run_and_parse_json()` is nice and elegant.
Copy link
Member

@HuijingHei HuijingHei left a comment

Choose a reason for hiding this comment

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

LGTM, so fast.

@HuijingHei
Copy link
Member

HuijingHei commented Jan 21, 2025

Maybe in future, also refactor the CommandRunExt into a separate internal crate and publish? Before that, agree it is the fastest way to use it.

@HuijingHei HuijingHei merged commit efe41b6 into coreos:main Jan 21, 2025
12 checks passed
@travier
Copy link
Member

travier commented Jan 21, 2025

Are we planning to publish this as an independent crate on crates.io? Otherwise I can see this causing problem with packaging in Fedora unless we vendor things which we don't do generally.

@cgwalters
Copy link
Member Author

Are we planning to publish this as an independent crate on crates.io?

Maybe.

Otherwise I can see this causing problem with packaging in Fedora unless we vendor things which we don't do generally.

Some projects for sure do, I think as you know I personally disagree with the approach for about 60% of the Fedora Rust packaging. And bootupd does vendor consistently across both Fedora+RHEL right now: https://src.fedoraproject.org/rpms/rust-bootupd/blob/rawhide/f/rust-bootupd.spec#_13

@travier
Copy link
Member

travier commented Jan 22, 2025

I disagree as well with the no-vendoring approach. I hadn't checked that we already vendor the code so that should not be an issue. Thanks

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