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

A few misc cleanups #746

Merged
merged 7 commits into from
Aug 1, 2024
Merged

Conversation

cgwalters
Copy link
Collaborator

@cgwalters cgwalters commented Jul 31, 2024

blockdev: Drop dead code

We have no use for listing all devices, so drop it.

Signed-off-by: Colin Walters [email protected]


blockdev: Fold listing fn

We only have a case for listing one device.

Signed-off-by: Colin Walters [email protected]


lib: Move Command extensions to new mod cmdutil

Prep for more stuff there.

Signed-off-by: Colin Walters [email protected]


cmdutils: Add helper to run and parse JSON

It's handy to have a central helper for this.

Signed-off-by: Colin Walters [email protected]


blockdev: Use JSON parsing helper

It's shorter and clearer and we get stderr.

Signed-off-by: Colin Walters [email protected]


Switch a few Task users to direct Command

I never intended the Task abstraction to be "the one way to
run subprocesses in bootc"...let's stop using it where:

  • We're using .quiet() by default so we're not printing the
    description anyways, which is the main thing Task does
  • We want to parse JSON, where we have a nice new helper

Signed-off-by: Colin Walters [email protected]


Fix two minor clippy lints

Signed-off-by: Colin Walters [email protected]


@github-actions github-actions bot added the area/install Issues related to `bootc install` label Jul 31, 2024
We have no use for listing all devices, so drop it.

Signed-off-by: Colin Walters <[email protected]>
We only have a case for listing one device.

Signed-off-by: Colin Walters <[email protected]>
Prep for more stuff there.

Signed-off-by: Colin Walters <[email protected]>
It's handy to have a central helper for this.

Signed-off-by: Colin Walters <[email protected]>
It's shorter and clearer and we get stderr.

Signed-off-by: Colin Walters <[email protected]>
I never intended the `Task` abstraction to be "the one way to
run subprocesses in bootc"...let's stop using it where:

- We're using `.quiet()` by default so we're not printing the
  description anyways, which is the *main* thing Task does
- We want to parse JSON, where we have a nice new helper

Signed-off-by: Colin Walters <[email protected]>
Signed-off-by: Colin Walters <[email protected]>
Copy link
Member

@jmarrero jmarrero left a comment

Choose a reason for hiding this comment

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

lgtm

@jmarrero
Copy link
Member

failure on x86_64:

               ERROR Switching: Pulling: Importing: Parsing layer blob sha256:ae48c94f9cd9579131fc1adf8d4ba22cdeab41348378b66399e53e265031ce8e: Failed to invoke skopeo proxy method FinishPipe: remote error: write |1: broken pipe
                    Shared connection to 127.0.0.1 closed.

I think that is likely a flake as it passed on aarch64...

@cgwalters
Copy link
Collaborator Author

Yep #579 is tracking that

@cgwalters cgwalters merged commit 0610971 into containers:main Aug 1, 2024
25 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/install Issues related to `bootc install`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants